This revision was automatically updated to reflect the committed changes. Closed by commit rGebb071345cda: [lldb/Core] Fix a race in the Communication class (authored by labath).
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77295/new/ https://reviews.llvm.org/D77295 Files: lldb/source/Core/Communication.cpp lldb/unittests/Core/CMakeLists.txt lldb/unittests/Core/CommunicationTest.cpp
Index: lldb/unittests/Core/CommunicationTest.cpp =================================================================== --- /dev/null +++ lldb/unittests/Core/CommunicationTest.cpp @@ -0,0 +1,35 @@ +//===-- 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()); +} Index: lldb/unittests/Core/CMakeLists.txt =================================================================== --- lldb/unittests/Core/CMakeLists.txt +++ lldb/unittests/Core/CMakeLists.txt @@ -1,4 +1,5 @@ add_lldb_unittest(LLDBCoreTests + CommunicationTest.cpp MangledTest.cpp RichManglingContextTest.cpp StreamCallbackTest.cpp Index: lldb/source/Core/Communication.cpp =================================================================== --- lldb/source/Core/Communication.cpp +++ lldb/source/Core/Communication.cpp @@ -315,16 +315,12 @@ 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) + if (bytes_read > 0 || status == eConnectionStatusEndOfFile) 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: @@ -332,11 +328,12 @@ 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 - comm->Disconnect(); + disconnect = comm->GetCloseOnEOF(); done = true; } if (error.Fail()) @@ -365,9 +362,15 @@ if (log) LLDB_LOGF(log, "%p Communication::ReadThread () thread exiting...", p); - comm->m_read_thread_did_exit = true; - // Let clients know that this thread is exiting comm->BroadcastEvent(eBroadcastBitNoMorePendingInput); + { + std::lock_guard<std::mutex> guard(comm->m_synchronize_mutex); + comm->m_read_thread_did_exit = true; + if (disconnect) + comm->Disconnect(); + } + + // Let clients know that this thread is exiting comm->BroadcastEvent(eBroadcastBitReadThreadDidExit); return {}; }
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
