[Lldb-commits] [PATCH] D11296: [LLGS] Get rid of the stdio forwarding thread

[Pavel Labath]

labath added inline comments.

================
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:940
@@ -957,1 +939,3 @@
+        if (! m_stdio_handle_up)
+            return error;
     }
----------------
ovyalov wrote:
> m_stdio_communication.Disconnect() ?
Added. Note that this will probably send a SIGHUP to the inferior. However, at this point, I don't think we care about that.

================
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:967
@@ +966,3 @@
+                log->Printf("GDBRemoteCommunicationServerLLGS::%s Stopping stdio forwarding as communication returned status %d (error: %s)", __FUNCTION__, status, error.AsCString());
+            m_stdio_handle_up.reset();
+            return;
----------------
ovyalov wrote:
> If SendProcessOutput is called from ProcessStateChanged within Monitor thread context we may have a race here:
>  - simultaneous access to m_stdio_handle_up from main & monitor threads.
>  - MainLoop will be accessed from multiple threads because UnregisterReadObject is called from monitor.
Nice catch. For the first problem, I propose to extend the scope of the `m_stdio_communication_mutex` to cover `m_stdio_handle_up` as well (most of the uses are covered by it already). For the second issue, I will prepare a separate change to make MainLoop thread-safe.


http://reviews.llvm.org/D11296