Author: Pavel Labath Date: 2020-04-09T12:49:56+02:00 New Revision: 76975c744da1e82ca6bdbe6883c799340acaf4f0
URL: https://github.com/llvm/llvm-project/commit/76975c744da1e82ca6bdbe6883c799340acaf4f0 DIFF: https://github.com/llvm/llvm-project/commit/76975c744da1e82ca6bdbe6883c799340acaf4f0.diff LOG: Revert "[lldb/Core] Fix a race in the Communication class" This reverts commit ebb071345cdae2509c55f9eec76090926fee86a2 -- it seems to introduce a deadlock in some circumstances. Added: Modified: lldb/source/Core/Communication.cpp lldb/unittests/Core/CMakeLists.txt Removed: lldb/unittests/Core/CommunicationTest.cpp ################################################################################ diff --git a/lldb/source/Core/Communication.cpp b/lldb/source/Core/Communication.cpp index c3e421191b01..f4163847e4bb 100644 --- a/lldb/source/Core/Communication.cpp +++ b/lldb/source/Core/Communication.cpp @@ -315,12 +315,16 @@ lldb::thread_result_t Communication::ReadThread(lldb::thread_arg_t p) { Status error; ConnectionStatus status = eConnectionStatusSuccess; bool done = false; - bool disconnect = false; while (!done && comm->m_read_thread_enabled) { size_t bytes_read = comm->ReadFromConnection( buf, sizeof(buf), std::chrono::seconds(5), status, &error); - if (bytes_read > 0 || status == eConnectionStatusEndOfFile) + if (bytes_read > 0) comm->AppendBytesToCache(buf, bytes_read, true, status); + else if ((bytes_read == 0) && status == eConnectionStatusEndOfFile) { + if (comm->GetCloseOnEOF()) + comm->Disconnect(); + comm->AppendBytesToCache(buf, bytes_read, true, status); + } switch (status) { case eConnectionStatusSuccess: @@ -328,12 +332,11 @@ lldb::thread_result_t Communication::ReadThread(lldb::thread_arg_t p) { case eConnectionStatusEndOfFile: done = true; - disconnect = comm->GetCloseOnEOF(); break; case eConnectionStatusError: // Check GetError() for details if (error.GetType() == eErrorTypePOSIX && error.GetError() == EIO) { // EIO on a pipe is usually caused by remote shutdown - disconnect = comm->GetCloseOnEOF(); + comm->Disconnect(); done = true; } if (error.Fail()) @@ -362,15 +365,9 @@ lldb::thread_result_t Communication::ReadThread(lldb::thread_arg_t p) { if (log) LLDB_LOGF(log, "%p Communication::ReadThread () thread exiting...", p); - comm->BroadcastEvent(eBroadcastBitNoMorePendingInput); - { - std::lock_guard<std::mutex> guard(comm->m_synchronize_mutex); - comm->m_read_thread_did_exit = true; - if (disconnect) - comm->Disconnect(); - } - + comm->m_read_thread_did_exit = true; // Let clients know that this thread is exiting + comm->BroadcastEvent(eBroadcastBitNoMorePendingInput); comm->BroadcastEvent(eBroadcastBitReadThreadDidExit); return {}; } diff --git a/lldb/unittests/Core/CMakeLists.txt b/lldb/unittests/Core/CMakeLists.txt index 6393c6fe38c2..688a39d9a10f 100644 --- a/lldb/unittests/Core/CMakeLists.txt +++ b/lldb/unittests/Core/CMakeLists.txt @@ -1,5 +1,4 @@ add_lldb_unittest(LLDBCoreTests - CommunicationTest.cpp MangledTest.cpp RichManglingContextTest.cpp StreamCallbackTest.cpp diff --git a/lldb/unittests/Core/CommunicationTest.cpp b/lldb/unittests/Core/CommunicationTest.cpp deleted file mode 100644 index 6ad0bc720d93..000000000000 --- a/lldb/unittests/Core/CommunicationTest.cpp +++ /dev/null @@ -1,35 +0,0 @@ -//===-- CommunicationTest.cpp ---------------------------------------------===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - -#include "lldb/Core/Communication.h" -#include "lldb/Host/ConnectionFileDescriptor.h" -#include "lldb/Host/Pipe.h" -#include "llvm/Testing/Support/Error.h" -#include "gtest/gtest.h" - -using namespace lldb_private; - -TEST(CommunicationTest, SynchronizeWhileClosing) { - // Set up a communication object reading from a pipe. - Pipe pipe; - ASSERT_THAT_ERROR(pipe.CreateNew(/*child_process_inherit=*/false).ToError(), - llvm::Succeeded()); - - Communication comm("test"); - comm.SetConnection(std::make_unique<ConnectionFileDescriptor>( - pipe.ReleaseReadFileDescriptor(), /*owns_fd=*/true)); - comm.SetCloseOnEOF(true); - ASSERT_TRUE(comm.StartReadThread()); - - // Ensure that we can safely synchronize with the read thread while it is - // closing the read end (in response to us closing the write end). - pipe.CloseWriteFileDescriptor(); - comm.SynchronizeWithReadThread(); - - ASSERT_TRUE(comm.StopReadThread()); -} _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits