[Lldb-commits] [lldb] r149346 - in /lldb/tags/lldb-109: include/lldb/Core/ConnectionFileDescriptor.h include/lldb/Target/Thread.h include/lldb/Target/ThreadList.h source/API/SBValue.cpp source/Core/Communication.cpp source/Core/ConnectionFileDescriptor.cpp source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp source/Target/Process.cpp source/Target/ThreadList.cpp tools/driver/Driver.cpp
Greg Clayton
gclayton at apple.com
Mon Jan 30 20:35:00 PST 2012
Author: gclayton
Date: Mon Jan 30 22:35:00 2012
New Revision: 149346
URL: http://llvm.org/viewvc/llvm-project?rev=149346&view=rev
Log:
<rdar://problem/10760649>
Fixed an issue with closing debug sessions down where we might end up
double closing a file descriptor. Previously we had a mutex in a class
that was being used in a read thread, and the main thread would come along
and cancel the thread during the file close and the thread would exit without
releasing the mutex and cause deadlocks. I solved the issue by not canceling
the read thread and closing all needed file descriptors.
Modified:
lldb/tags/lldb-109/include/lldb/Core/ConnectionFileDescriptor.h
lldb/tags/lldb-109/include/lldb/Target/Thread.h
lldb/tags/lldb-109/include/lldb/Target/ThreadList.h
lldb/tags/lldb-109/source/API/SBValue.cpp
lldb/tags/lldb-109/source/Core/Communication.cpp
lldb/tags/lldb-109/source/Core/ConnectionFileDescriptor.cpp
lldb/tags/lldb-109/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
lldb/tags/lldb-109/source/Target/Process.cpp
lldb/tags/lldb-109/source/Target/ThreadList.cpp
lldb/tags/lldb-109/tools/driver/Driver.cpp
Modified: lldb/tags/lldb-109/include/lldb/Core/ConnectionFileDescriptor.h
URL: http://llvm.org/viewvc/llvm-project/lldb/tags/lldb-109/include/lldb/Core/ConnectionFileDescriptor.h?rev=149346&r1=149345&r2=149346&view=diff
==============================================================================
--- lldb/tags/lldb-109/include/lldb/Core/ConnectionFileDescriptor.h (original)
+++ lldb/tags/lldb-109/include/lldb/Core/ConnectionFileDescriptor.h Mon Jan 30 22:35:00 2012
@@ -105,7 +105,7 @@
SocketAddress m_udp_send_sockaddr;
bool m_should_close_fd; // True if this class should close the file descriptor when it goes away.
uint32_t m_socket_timeout_usec;
- //Mutex m_mutex;
+ Mutex m_mutex;
static in_port_t
GetSocketPort (int fd);
Modified: lldb/tags/lldb-109/include/lldb/Target/Thread.h
URL: http://llvm.org/viewvc/llvm-project/lldb/tags/lldb-109/include/lldb/Target/Thread.h?rev=149346&r1=149345&r2=149346&view=diff
==============================================================================
--- lldb/tags/lldb-109/include/lldb/Target/Thread.h (original)
+++ lldb/tags/lldb-109/include/lldb/Target/Thread.h Mon Jan 30 22:35:00 2012
@@ -769,6 +769,7 @@
protected:
friend class ThreadPlan;
+ friend class ThreadList;
friend class StackFrameList;
// This is necessary to make sure thread assets get destroyed while the thread is still in good shape
Modified: lldb/tags/lldb-109/include/lldb/Target/ThreadList.h
URL: http://llvm.org/viewvc/llvm-project/lldb/tags/lldb-109/include/lldb/Target/ThreadList.h?rev=149346&r1=149345&r2=149346&view=diff
==============================================================================
--- lldb/tags/lldb-109/include/lldb/Target/ThreadList.h (original)
+++ lldb/tags/lldb-109/include/lldb/Target/ThreadList.h Mon Jan 30 22:35:00 2012
@@ -59,6 +59,9 @@
void
Clear();
+ void
+ Destroy();
+
// Note that "idx" is not the same as the "thread_index". It is a zero
// based index to accessing the current threads, whereas "thread_index"
// is a unique index assigned
Modified: lldb/tags/lldb-109/source/API/SBValue.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/tags/lldb-109/source/API/SBValue.cpp?rev=149346&r1=149345&r2=149346&view=diff
==============================================================================
--- lldb/tags/lldb-109/source/API/SBValue.cpp (original)
+++ lldb/tags/lldb-109/source/API/SBValue.cpp Mon Jan 30 22:35:00 2012
@@ -375,7 +375,10 @@
lldb::SBValue
SBValue::Cast (SBType type)
{
- return CreateChildAtOffset(m_opaque_sp->GetName().GetCString(), 0, type);
+ lldb::SBValue sb_value;
+ if (m_opaque_sp)
+ sb_value = CreateChildAtOffset(m_opaque_sp->GetName().GetCString(), 0, type);
+ return sb_value;
}
lldb::SBValue
@@ -1146,14 +1149,14 @@
SBValue::Watch (bool resolve_location, bool read, bool write)
{
lldb::SBWatchpoint sb_watchpoint;
- if (!m_opaque_sp)
- return sb_watchpoint;
-
- Target* target = m_opaque_sp->GetUpdatePoint().GetTargetSP().get();
- if (target)
+ if (m_opaque_sp)
{
- Mutex::Locker api_locker (target->GetAPIMutex());
- sb_watchpoint = WatchValue(read, write, false);
+ Target* target = m_opaque_sp->GetUpdatePoint().GetTargetSP().get();
+ if (target)
+ {
+ Mutex::Locker api_locker (target->GetAPIMutex());
+ sb_watchpoint = WatchValue(read, write, false);
+ }
}
LogSP log(GetLogIfAllCategoriesSet (LIBLLDB_LOG_API));
if (log)
@@ -1166,14 +1169,14 @@
SBValue::WatchPointee (bool resolve_location, bool read, bool write)
{
lldb::SBWatchpoint sb_watchpoint;
- if (!m_opaque_sp)
- return sb_watchpoint;
-
- Target* target = m_opaque_sp->GetUpdatePoint().GetTargetSP().get();
- if (target)
+ if (m_opaque_sp)
{
- Mutex::Locker api_locker (target->GetAPIMutex());
- sb_watchpoint = WatchValue(read, write, true);
+ Target* target = m_opaque_sp->GetUpdatePoint().GetTargetSP().get();
+ if (target)
+ {
+ Mutex::Locker api_locker (target->GetAPIMutex());
+ sb_watchpoint = WatchValue(read, write, true);
+ }
}
LogSP log(GetLogIfAllCategoriesSet (LIBLLDB_LOG_API));
if (log)
Modified: lldb/tags/lldb-109/source/Core/Communication.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/tags/lldb-109/source/Core/Communication.cpp?rev=149346&r1=149345&r2=149346&view=diff
==============================================================================
--- lldb/tags/lldb-109/source/Core/Communication.cpp (original)
+++ lldb/tags/lldb-109/source/Core/Communication.cpp Mon Jan 30 22:35:00 2012
@@ -65,8 +65,8 @@
Communication::Clear()
{
SetReadThreadBytesReceivedCallback (NULL, NULL);
- StopReadThread (NULL);
Disconnect (NULL);
+ StopReadThread (NULL);
}
ConnectionStatus
@@ -253,7 +253,7 @@
BroadcastEvent (eBroadcastBitReadThreadShouldExit, NULL);
- Host::ThreadCancel (m_read_thread, error_ptr);
+ //Host::ThreadCancel (m_read_thread, error_ptr);
bool status = Host::ThreadJoin (m_read_thread, NULL, error_ptr);
m_read_thread = LLDB_INVALID_HOST_THREAD;
@@ -383,7 +383,8 @@
// Let clients know that this thread is exiting
comm->BroadcastEvent (eBroadcastBitReadThreadDidExit);
comm->m_read_thread_enabled = false;
- comm->Disconnect();
+ comm->m_read_thread = LLDB_INVALID_HOST_THREAD;
+// comm->Disconnect();
return NULL;
}
@@ -401,8 +402,8 @@
void
Communication::SetConnection (Connection *connection)
{
- StopReadThread(NULL);
Disconnect (NULL);
+ StopReadThread(NULL);
m_connection_sp.reset(connection);
}
Modified: lldb/tags/lldb-109/source/Core/ConnectionFileDescriptor.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/tags/lldb-109/source/Core/ConnectionFileDescriptor.cpp?rev=149346&r1=149345&r2=149346&view=diff
==============================================================================
--- lldb/tags/lldb-109/source/Core/ConnectionFileDescriptor.cpp (original)
+++ lldb/tags/lldb-109/source/Core/ConnectionFileDescriptor.cpp Mon Jan 30 22:35:00 2012
@@ -31,7 +31,7 @@
#include "lldb/Core/Log.h"
#include "lldb/Core/RegularExpression.h"
#include "lldb/Core/Timer.h"
-
+#include "lldb/Host/Host.h"
using namespace lldb;
using namespace lldb_private;
@@ -73,8 +73,8 @@
m_fd_recv_type (eFDTypeFile),
m_udp_send_sockaddr (),
m_should_close_fd (false),
- m_socket_timeout_usec(0)//,
- //m_mutex (Mutex::eMutexTypeRecursive)
+ m_socket_timeout_usec(0),
+ m_mutex (Mutex::eMutexTypeRecursive)
{
LogSP log(lldb_private::GetLogIfAnyCategoriesSet (LIBLLDB_LOG_CONNECTION | LIBLLDB_LOG_OBJECT));
if (log)
@@ -89,7 +89,8 @@
m_fd_recv_type (eFDTypeFile),
m_udp_send_sockaddr (),
m_should_close_fd (owns_fd),
- m_socket_timeout_usec(0)
+ m_socket_timeout_usec(0),
+ m_mutex (Mutex::eMutexTypeRecursive)
{
LogSP log(lldb_private::GetLogIfAnyCategoriesSet (LIBLLDB_LOG_CONNECTION | LIBLLDB_LOG_OBJECT));
if (log)
@@ -114,7 +115,7 @@
ConnectionStatus
ConnectionFileDescriptor::Connect (const char *s, Error *error_ptr)
{
- //Mutex::Locker locker (m_mutex);
+ Mutex::Locker locker (m_mutex);
LogSP log(lldb_private::GetLogIfAnyCategoriesSet (LIBLLDB_LOG_CONNECTION));
if (log)
log->Printf ("%p ConnectionFileDescriptor::Connect (url = '%s')", this, s);
@@ -234,6 +235,7 @@
ConnectionStatus
ConnectionFileDescriptor::Disconnect (Error *error_ptr)
{
+ Mutex::Locker locker (m_mutex);
LogSP log(lldb_private::GetLogIfAnyCategoriesSet (LIBLLDB_LOG_CONNECTION));
if (log)
log->Printf ("%p ConnectionFileDescriptor::Disconnect ()", this);
@@ -243,22 +245,25 @@
return eConnectionStatusSuccess;
}
ConnectionStatus status = eConnectionStatusSuccess;
- if (m_fd_send == m_fd_recv)
- {
- // Both file descriptors are the same, only close one
- status = Close (m_fd_send, error_ptr);
- m_fd_recv = -1;
- }
- else
+ if (m_fd_send >= 0 || m_fd_recv >= 0)
{
- // File descriptors are the different, close both if needed
- if (m_fd_send >= 0)
+ if (m_fd_send == m_fd_recv)
+ {
+ // Both file descriptors are the same, only close one
+ m_fd_recv = -1;
status = Close (m_fd_send, error_ptr);
- if (m_fd_recv >= 0)
+ }
+ else
{
- ConnectionStatus recv_status = Close (m_fd_recv, error_ptr);
- if (status == eConnectionStatusSuccess)
- status = recv_status;
+ // File descriptors are the different, close both if needed
+ if (m_fd_send >= 0)
+ status = Close (m_fd_send, error_ptr);
+ if (m_fd_recv >= 0)
+ {
+ ConnectionStatus recv_status = Close (m_fd_recv, error_ptr);
+ if (status == eConnectionStatusSuccess)
+ status = recv_status;
+ }
}
}
return status;
@@ -598,30 +603,60 @@
return eConnectionStatusLostConnection;
}
+//class ScopedPThreadCancelDisabler
+//{
+//public:
+// ScopedPThreadCancelDisabler()
+// {
+// // Disable the ability for this thread to be cancelled
+// int err = ::pthread_setcancelstate (PTHREAD_CANCEL_DISABLE, &m_old_state);
+// if (err != 0)
+// m_old_state = -1;
+//
+// }
+//
+// ~ScopedPThreadCancelDisabler()
+// {
+// // Restore the ability for this thread to be cancelled to what it
+// // previously was.
+// if (m_old_state != -1)
+// ::pthread_setcancelstate (m_old_state, 0);
+// }
+//private:
+// int m_old_state; // Save the old cancelability state.
+//};
+//
ConnectionStatus
ConnectionFileDescriptor::Close (int& fd, Error *error_ptr)
{
- //Mutex::Locker locker (m_mutex);
+ //ScopedPThreadCancelDisabler pthread_cancel_disabler;
if (error_ptr)
error_ptr->Clear();
bool success = true;
+ // Avoid taking a lock if we can
if (fd >= 0)
{
- LogSP log(lldb_private::GetLogIfAnyCategoriesSet (LIBLLDB_LOG_CONNECTION));
- if (log)
- log->Printf ("%p ConnectionFileDescriptor::Close (fd = %i)", this,fd);
-
- success = ::close (fd) == 0;
- if (!success && error_ptr)
- {
- // Only set the error if we have been asked to since something else
- // might have caused us to try and shut down the connection and may
- // have already set the error.
- error_ptr->SetErrorToErrno();
+ Mutex::Locker locker (m_mutex);
+ // Check the FD after the lock is taken to ensure only one thread
+ // can get into the close scope below
+ if (fd >= 0)
+ {
+ LogSP log(lldb_private::GetLogIfAnyCategoriesSet (LIBLLDB_LOG_CONNECTION));
+ if (log)
+ log->Printf ("%p ConnectionFileDescriptor::Close (fd = %i)", this,fd);
+
+ success = ::close (fd) == 0;
+ // A reference to a FD was passed in, set it to an invalid value
+ fd = -1;
+ if (!success && error_ptr)
+ {
+ // Only set the error if we have been asked to since something else
+ // might have caused us to try and shut down the connection and may
+ // have already set the error.
+ error_ptr->SetErrorToErrno();
+ }
}
- fd = -1;
}
- m_fd_send_type = m_fd_recv_type = eFDTypeFile;
if (success)
return eConnectionStatusSuccess;
else
Modified: lldb/tags/lldb-109/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/tags/lldb-109/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp?rev=149346&r1=149345&r2=149346&view=diff
==============================================================================
--- lldb/tags/lldb-109/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp (original)
+++ lldb/tags/lldb-109/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp Mon Jan 30 22:35:00 2012
@@ -469,8 +469,7 @@
// Sleep for one second to let the process get all detached...
StopAsyncThread ();
- m_comm.StopReadThread();
- m_comm.Disconnect(); // Disconnect from the debug server.
+ m_comm.Clear();
SetPrivateState (eStateDetached);
ResumePrivateStateThread();
@@ -508,8 +507,7 @@
}
}
StopAsyncThread ();
- m_comm.StopReadThread();
- m_comm.Disconnect(); // Disconnect from the debug server.
+ m_comm.Clear();
return error;
}
Modified: lldb/tags/lldb-109/source/Target/Process.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/tags/lldb-109/source/Target/Process.cpp?rev=149346&r1=149345&r2=149346&view=diff
==============================================================================
--- lldb/tags/lldb-109/source/Target/Process.cpp (original)
+++ lldb/tags/lldb-109/source/Target/Process.cpp Mon Jan 30 22:35:00 2012
@@ -848,7 +848,7 @@
m_abi_sp.reset();
m_os_ap.reset();
m_dyld_ap.reset();
- m_thread_list.Clear();
+ m_thread_list.Destroy();
std::vector<Notifications> empty_notifications;
m_notifications.swap(empty_notifications);
m_image_tokens.clear();
@@ -2794,8 +2794,7 @@
DidDestroy();
StopPrivateStateThread();
}
- m_stdio_communication.StopReadThread();
- m_stdio_communication.Disconnect();
+ m_stdio_communication.Clear();
if (m_process_input_reader && m_process_input_reader->IsActive())
m_target.GetDebugger().PopInputReader (m_process_input_reader);
if (m_process_input_reader)
Modified: lldb/tags/lldb-109/source/Target/ThreadList.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/tags/lldb-109/source/Target/ThreadList.cpp?rev=149346&r1=149345&r2=149346&view=diff
==============================================================================
--- lldb/tags/lldb-109/source/Target/ThreadList.cpp (original)
+++ lldb/tags/lldb-109/source/Target/ThreadList.cpp Mon Jan 30 22:35:00 2012
@@ -355,6 +355,17 @@
}
void
+ThreadList::Destroy()
+{
+ Mutex::Locker locker(m_threads_mutex);
+ const uint32_t num_threads = m_threads.size();
+ for (uint32_t idx = 0; idx < num_threads; ++idx)
+ {
+ m_threads[idx]->DestroyThread();
+ }
+}
+
+void
ThreadList::RefreshStateAfterStop ()
{
Mutex::Locker locker(m_threads_mutex);
@@ -603,6 +614,32 @@
m_stop_id = rhs.m_stop_id;
m_threads.swap(rhs.m_threads);
m_selected_tid = rhs.m_selected_tid;
+
+
+ // Now we look for threads that we are done with and
+ // make sure to clear them up as much as possible so
+ // anyone with a shared pointer will still have a reference,
+ // but the thread won't be of much use. Using std::weak_ptr
+ // for all backward references (such as a thread to a process)
+ // will eventually solve this issue for us, but for now, we
+ // need to work around the issue
+ collection::iterator rhs_pos, rhs_end = rhs.m_threads.end();
+ for (rhs_pos = rhs.m_threads.begin(); rhs_pos != rhs_end; ++rhs_pos)
+ {
+ const lldb::tid_t tid = (*rhs_pos)->GetID();
+ bool thread_is_alive = false;
+ const uint32_t num_threads = m_threads.size();
+ for (uint32_t idx = 0; idx < num_threads; ++idx)
+ {
+ if (m_threads[idx]->GetID() == tid)
+ {
+ thread_is_alive = true;
+ break;
+ }
+ }
+ if (!thread_is_alive)
+ (*rhs_pos)->DestroyThread();
+ }
}
}
Modified: lldb/tags/lldb-109/tools/driver/Driver.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/tags/lldb-109/tools/driver/Driver.cpp?rev=149346&r1=149345&r2=149346&view=diff
==============================================================================
--- lldb/tags/lldb-109/tools/driver/Driver.cpp (original)
+++ lldb/tags/lldb-109/tools/driver/Driver.cpp Mon Jan 30 22:35:00 2012
@@ -1347,7 +1347,13 @@
else if (event.BroadcasterMatchesRef (sb_interpreter.GetBroadcaster()))
{
if (event_type & SBCommandInterpreter::eBroadcastBitQuitCommandReceived)
+ {
+ editline_output_pty.CloseMasterFileDescriptor();
+ master_out_comm.Disconnect();
+ out_comm_2.Disconnect();
+ fclose (stdin);
done = true;
+ }
else if (event_type & SBCommandInterpreter::eBroadcastBitAsynchronousErrorData)
{
const char *data = SBEvent::GetCStringFromEvent (event);
More information about the lldb-commits
mailing list