From ab728678ee47ca578b964af77315dafd70688cb1 Mon Sep 17 00:00:00 2001 From: Jim Ingham Date: Tue, 17 Jul 2012 01:47:11 +0000 Subject: [PATCH] Lock around reading as well as connecting & disconnecting so we don't start reading after someone has set our file descriptor to -1 and crash in FD_SET... llvm-svn: 160336 --- .../lldb/Core/ConnectionFileDescriptor.h | 8 +- lldb/source/Core/ConnectionFileDescriptor.cpp | 104 ++++++++++++------ 2 files changed, 78 insertions(+), 34 deletions(-) diff --git a/lldb/include/lldb/Core/ConnectionFileDescriptor.h b/lldb/include/lldb/Core/ConnectionFileDescriptor.h index f7710718b8b2..78e4ec4775ce 100644 --- a/lldb/include/lldb/Core/ConnectionFileDescriptor.h +++ b/lldb/include/lldb/Core/ConnectionFileDescriptor.h @@ -112,9 +112,11 @@ protected: 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; - int m_command_fd_send; // A pipe that we select on the reading end of along with - int m_command_fd_receive; // m_fd_recv so we can force ourselves out of the select. - Mutex m_mutex; + int m_command_fd_send; // A pipe that we select on the reading end of along with + int m_command_fd_receive; // m_fd_recv so we can force ourselves out of the select. + Mutex m_mutex; + bool m_shutting_down; // This marks that we are shutting down so if we get woken up from BytesAvailable + // to disconnect, we won't try to read again. static in_port_t GetSocketPort (int fd); diff --git a/lldb/source/Core/ConnectionFileDescriptor.cpp b/lldb/source/Core/ConnectionFileDescriptor.cpp index 0345def0ac58..3f0533cbd998 100644 --- a/lldb/source/Core/ConnectionFileDescriptor.cpp +++ b/lldb/source/Core/ConnectionFileDescriptor.cpp @@ -77,7 +77,8 @@ ConnectionFileDescriptor::ConnectionFileDescriptor () : m_socket_timeout_usec(0), m_command_fd_send(-1), m_command_fd_receive(-1), - m_mutex (Mutex::eMutexTypeRecursive) + m_mutex (Mutex::eMutexTypeRecursive), + m_shutting_down (false) { LogSP log(lldb_private::GetLogIfAnyCategoriesSet (LIBLLDB_LOG_CONNECTION | LIBLLDB_LOG_OBJECT)); if (log) @@ -95,7 +96,8 @@ ConnectionFileDescriptor::ConnectionFileDescriptor (int fd, bool owns_fd) : m_socket_timeout_usec(0), m_command_fd_send(-1), m_command_fd_receive(-1), - m_mutex (Mutex::eMutexTypeRecursive) + m_mutex (Mutex::eMutexTypeRecursive), + m_shutting_down (false) { LogSP log(lldb_private::GetLogIfAnyCategoriesSet (LIBLLDB_LOG_CONNECTION | LIBLLDB_LOG_OBJECT)); if (log) @@ -282,50 +284,72 @@ ConnectionFileDescriptor::Connect (const char *s, Error *error_ptr) 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); ConnectionStatus status = eConnectionStatusSuccess; - if (m_fd_send >= 0 || m_fd_recv >= 0) + if (m_fd_send < 0 && m_fd_recv < 0) { - if (m_should_close_fd == false) + if (log) + log->Printf ("%p ConnectionFileDescriptor::Disconnect(): Nothing to disconnect", this); + return eConnectionStatusSuccess; + } + + // Try to get the ConnectionFileDescriptor's mutex. If we fail, that is quite likely + // because somebody is doing a blocking read on our file descriptor. If that's the case, + // then send the "q" char to the command file channel so the read will wake up and the connection + // will then know to shut down. + + m_shutting_down = true; + + Mutex::Locker locker; + bool got_lock= locker.TryLock (m_mutex); + + if (!got_lock) + { + if (m_command_fd_send != -1 ) { - m_fd_send = m_fd_recv = -1; + write (m_command_fd_send, "q", 1); + close (m_command_fd_send); + m_command_fd_send = -1; + } + locker.Lock (m_mutex); + } + + if (m_should_close_fd == true) + { + if (m_fd_send == m_fd_recv) + { + status = Close (m_fd_send, error_ptr); } else { - if (m_fd_send == m_fd_recv) - { - // Both file descriptors are the same, only close one - m_fd_recv = -1; + // File descriptors are the different, close both if needed + if (m_fd_send >= 0) status = Close (m_fd_send, error_ptr); - } - else + if (m_fd_recv >= 0) { - // 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; - } + ConnectionStatus recv_status = Close (m_fd_recv, error_ptr); + if (status == eConnectionStatusSuccess) + status = recv_status; } } + } + + // Now set all our descriptors to invalid values. + + m_fd_send = m_fd_recv = -1; + + if (status != eConnectionStatusSuccess) + { - // Now write a byte to the command pipe to wake our Reader if it is stuck in read. - if (m_command_fd_send != -1 ) - { - write (m_command_fd_send, "q", 1); - close (m_command_fd_send); - m_command_fd_send = -1; - } + return status; } - return status; + + m_shutting_down = false; + return eConnectionStatusSuccess; } size_t @@ -340,6 +364,22 @@ ConnectionFileDescriptor::Read (void *dst, log->Printf ("%p ConnectionFileDescriptor::Read () ::read (fd = %i, dst = %p, dst_len = %zu)...", this, m_fd_recv, dst, dst_len); + Mutex::Locker locker; + bool got_lock = locker.TryLock (m_mutex); + if (!got_lock) + { + if (log) + log->Printf ("%p ConnectionFileDescriptor::Read () failed to get the connection lock.", + this); + if (error_ptr) + error_ptr->SetErrorString ("failed to get the connection lock for read."); + + status = eConnectionStatusTimedOut; + return 0; + } + else if (m_shutting_down) + return eConnectionStatusError; + ssize_t bytes_read = 0; status = BytesAvailable (timeout_usec, error_ptr); @@ -542,7 +582,6 @@ ConnectionFileDescriptor::Write (const void *src, size_t src_len, ConnectionStat break; // Break to close.... } - //Disconnect (NULL); return 0; } @@ -553,6 +592,9 @@ ConnectionFileDescriptor::Write (const void *src, size_t src_len, ConnectionStat ConnectionStatus ConnectionFileDescriptor::BytesAvailable (uint32_t timeout_usec, Error *error_ptr) { + // Don't need to take the mutex here separately since we are only called from Read. If we + // ever get used more generally we will need to lock here as well. + LogSP log(lldb_private::GetLogIfAnyCategoriesSet (LIBLLDB_LOG_CONNECTION)); if (log) log->Printf("%p ConnectionFileDescriptor::BytesAvailable (timeout_usec = %u)", this, timeout_usec); @@ -641,7 +683,7 @@ ConnectionFileDescriptor::BytesAvailable (uint32_t timeout_usec, Error *error_pt if (log) log->Printf("%p ConnectionFileDescriptor::BytesAvailable() got data: %*s from the command channel.", this, (int) bytes_read, buffer); - + return eConnectionStatusEndOfFile; } else -- GitLab