[all-commits] [llvm/llvm-project] 871f48: [lldb-dap] Fix a race during shutdown (#91591)

[Pavel Labath via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 871f4839f988a1ef59ea0371e0f25c8651a899f2
      https://github.com/llvm/llvm-project/commit/871f4839f988a1ef59ea0371e0f25c8651a899f2
  Author: Pavel Labath <pavel at labath.sk>
  Date:   2024-05-10 (Fri, 10 May 2024)

  Changed paths:
    M lldb/tools/lldb-dap/DAP.cpp
    M lldb/tools/lldb-dap/DAP.h
    M lldb/tools/lldb-dap/lldb-dap.cpp

  Log Message:
  -----------
  [lldb-dap] Fix a race during shutdown (#91591)

lldb-dap was setting a flag which was meant to shut it down as soon as
it sent a terminated event. The problem with this flag is two-fold:
- as far as I can tell (definitely not an expert here), there's no
justification for this in the protocol spec. The only way I found to
shut the server down was to send it a disconnect request.
- the flag did not actually work most of the time, because it's only
checked between requests so nothing will happen if the server starts
listening for a new request before a different thread manages to send
the terminated event. And since the next request is usually the
disconnect request, everything will operate normally.

The combination of these two things meant that the issue was largely
unnoticable, except for rare flaky test failures, which happened when
the handler thread was too slow, and checked the flag after it has
already been said. This caused the test suite to complain as it did not
get a response to the disconnect request. This situation could be
s(t)imulated by adding a sleep to the and of the main loop, which
delayed the flag check, and caused the DAP tests to fail reliably.

This patch changes the shutdown condition to only trigger when the
disconnect request has been received. Since the flag can now only be set
from the handler thread, it no longer needs to be atomic.



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications