llvm-capstone/lldb/source
Pavel Labath 769d7041cc Recommit "[lldb/Core] Fix a race in the Communication class"
The synchronization logic in the previous had a subtle bug. Moving of
the "m_read_thread_did_exit = true" into the critical section made it
possible for some threads calling SynchronizeWithReadThread call to get
stuck. This could happen if there were already past the point where they
checked this variable. In that case, they would block on waiting for the
eBroadcastBitNoMorePendingInput event, which would never come as the
read thread was blocked on getting the synchronization mutex.

The new version moves that line out of the critical section and before
the sending of the eBroadcastBitNoMorePendingInput event, and also adds
some comments to explain why the things need to be in this sequence:
- m_read_thread_did_exit = true: prevents new threads for waiting on
  events
- eBroadcastBitNoMorePendingInput: unblock any current thread waiting
  for the event
- Disconnect(): close the connection. This is the only bit that needs to
  be in the critical section, and this is to ensure that we don't close
  the connection while the synchronizing thread is mucking with it.

Original commit message follows:

Communication::SynchronizeWithReadThread is called whenever a process
stops to ensure that we process all of its stdout before we report the
stop. If the process exits, we first call this method, and then close
the connection.

However, when the child process exits, the thread reading its stdout
will usually (but not always) read an EOF because the other end of the
pty has been closed. In response to an EOF, the Communication read
thread closes it's end of the connection too.

This can result in a race where the read thread is closing the
connection while the synchronizing thread is attempting to get its
attention via Connection::InterruptRead.

The fix is to hold the synchronization mutex while closing the
connection.

I've found this issue while tracking down a rare flake in some of the
vscode tests. I am not sure this is the cause of those failures (as I
would have expected this issue to manifest itself differently), but it
is an issue nonetheless.

The attached test demonstrates the steps needed to reproduce the race.
It will fail under tsan without this patch.

Reviewers: clayborg, JDevlieghere

Subscribers: mgorny, lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D77295
2020-04-09 13:39:00 +02:00
..
API [commands] Support autorepeat in SBCommands 2020-04-08 10:54:14 -07:00
Breakpoint [lldb][nfc] early exit/continue 2020-04-03 14:50:08 +02:00
Commands [lldb] Fixing the bug that the "log timer" has no tab completion 2020-04-09 08:58:52 +02:00
Core Recommit "[lldb/Core] Fix a race in the Communication class" 2020-04-09 13:39:00 +02:00
DataFormatters [lldb][NFC] Always update m_cache_{hits/misses} in FormatCache 2020-03-24 20:16:43 +01:00
Expression [lldb] Add option to retry Fix-Its multiple times to failed expressions 2020-04-06 11:25:36 +02:00
Host Remap the target (Xcode) SDK directory to the host SDK directory. 2020-04-06 15:51:30 -07:00
Initialization [lldb][NFC] Fix all formatting errors in .cpp file headers 2020-01-24 08:52:55 +01:00
Interpreter [source maps] Fix remove, insert-after and replace 2020-04-03 19:15:56 -07:00
Plugins [DWARF] Not all the constant variables are "static". 2020-04-08 11:07:19 -07:00
Symbol [lldb] NFC: Fix trivial typo in comments, documents, and messages 2020-04-07 01:06:16 +09:00
Target Don't access reference to a vector after pop_back 2020-04-07 23:10:58 +02:00
Utility Remap the target (Xcode) SDK directory to the host SDK directory. 2020-04-06 15:51:30 -07:00
CMakeLists.txt Make LLVM_APPEND_VC_REV=OFF affect clang, lld, and lldb as well. 2020-01-16 19:04:08 -05:00
lldb.cpp [lldb] Fix version string when using LLDB_REVISION but not LLDB_REPOSITORY 2020-02-20 10:07:50 -08:00