[Lldb-commits] [PATCH] D77295: [lldb/Core] Fix a race in the Communication class

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.


================
Comment at: lldb/source/Core/Communication.cpp:318
   bool done = false;
+  bool disconnect = false;
   while (!done && comm->m_read_thread_enabled) {
----------------
Can we just init "disconnect" right here?:

```
const bool disconnect = comm->GetCloseOnEOF();
```



================
Comment at: lldb/source/Core/Communication.cpp:322-327
     if (bytes_read > 0)
       comm->AppendBytesToCache(buf, bytes_read, true, status);
     else if ((bytes_read == 0) && status == eConnectionStatusEndOfFile) {
-      if (comm->GetCloseOnEOF())
-        comm->Disconnect();
+      disconnect = comm->GetCloseOnEOF();
       comm->AppendBytesToCache(buf, bytes_read, true, status);
     }
----------------
Is there a reason we call AppendBytesToCache with zero bytes in the else? If we do need to call it (it might listen to the "status"?) then this:

```
if (bytes_read > 0)
  comm->AppendBytesToCache(buf, bytes_read, true, status);
else if ((bytes_read == 0) && status == eConnectionStatusEndOfFile) {
  disconnect = comm->GetCloseOnEOF();
  comm->AppendBytesToCache(buf, bytes_read, true, status);
}
```
can just be:

```
comm->AppendBytesToCache(buf, bytes_read, true, status);
```

bytes_read is a size_t so it can't be less than zero. Then we can move the "disconnect = comm->GetCloseOnEOF();" into the eConnectionStatusEndOfFile case in the switch below.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77295/new/

https://reviews.llvm.org/D77295