[Lldb-commits] [lldb] [lldb-dap] Add new optional argument `time-to-live` when using `--connection` (PR #156803)

[Roy Shi via lldb-commits]

================
@@ -327,6 +366,11 @@ serveConnection(const Socket::SocketProtocol &protocol, const std::string &name,
       std::unique_lock<std::mutex> lock(dap_sessions_mutex);
       dap_sessions.erase(&loop);
       std::notify_all_at_thread_exit(dap_sessions_condition, std::move(lock));
+
+      // Start the countdown to kill the server at the end of each connection.
----------------
royitaqi wrote:

@jeffreytan81:  Thanks for the suggestions / good points.

> I would suggest only schedule callback for the last connections alive

Got it. I think the optimization makes sense (as you said, the existing logic is still needed). I will try to use `dap_sessions` to detect "I was the last connection alive".

--

> there seems to be a race condition here
I can be wrong (new to `MainLoopBase` and `MainLoopPosix`).

**TL;DR**: IIUC, there is no such race condition. This is guaranteed by `MainLoopPosix`, because:
- It handles all callbacks and read objects in sequence (see `MainLoopPosix::Run()`).
- It checks `m_terminate_request` before processing each read object (see `MainLoopPosix::RunImpl::ProcessReadEvents()`).

**Details**:
So, either of the following will happen:

**Case 1: Last connection times out first**: In this case, the timeout callback invokes `loop.RequestTermination()` and sets `m_terminate_request`. Then, the socket detects a new connection, but the callback given to `Accept()` is never invoked, because `m_terminate_request` is already set and the object read from the socket is discarded.

**Case 2: New client connects first**: In this case, the callback given to `Accept()` is invoked. It will reset the global variable before spinning the rest of the initiation into a separate thread. Then, the "last" connection's timeout callback is invoked. It will see that the global variable has been reset, so it won't request termination.

Kindly LMK if I missed anything.

With that said, I will move the `loop.RequestTermination()` call into the scoped lock, because it's a cheap operation anyways.

https://github.com/llvm/llvm-project/pull/156803