Implement well defined window for breakpoint configuration. #313
Conversation
|
Thank you for this contribution, that looks nice and the defined window is much better than the manual event check. It looks like you've tested that already on GDB and LLDB, correct? |
Yes, that is correct. I ran launch and attach scenarios on both GDB and LLDB, and both seem to operate as expected. |
|
Could you do me a favor and try something that takes long to attach? I'd highly suggest attaching to a process with lots of loaded shared objects (maybe your browser, or something else) and/or attach via SSH to a gdbserver. |
|
actually after reviewing where the event is sent, this seems like it should never work like this?
If the initialized-event is sent on debug-ready, that should never actually start, as debug-ready is emitted when load/ssh/attach has been run (which in this picture corresponds to the launch request) |
|
Hi @WebFreak001, my interpretation of that UML sequence diagram is that the configuration happens asynchronously (per the dashed arrow on the "initialized event") from the primary focus of control in that lifeline. Therefore, I don't believe there is an implied order between the "launch request" and the breakpoint configuration shown above it. I think the diagram likely could have been constructed with the "launch request" shown above the breakpoint configuration. |
|
That was my interpretation that there was no ordering dependency there. That was further emphasized by the wording in the overview which reinforced this understanding:
|
|
before that however:
|
Yes, that is correct. This is accomplished by having the continue/run commands, which are sent to the debugger, gated by having received the configurationDone request (per here) which guarantees that all breakpoint configuration has occurred prior to program execution starting. |
|
ok I'd be fine with merging then if you remove the bind(this) and change it to |
Sounds good, thanks for taking a look at this. I'll make your suggested changes and push a new update. |
The previous implementation worked under the assumption that breakpoint requests would be received from the client prior to the debugger having completed it's initialization. However, if the debugger completed initialization prior to having received breakpoint requests from the client, those breakpoints requests would be ignored (i.e., there was a race condition). Additionally, the previous implementation "buffered" the receipt of the breakpoint requests and would only process them after the debugger initialization had completed (i.e., "debug-ready"). This change delays the sending of the "initialized" event back to the client (until the debugger truly has completed its initialization). As a result, there is no longer a need to buffer the breakpoint requests. Breakpoints requests are now forced to be received after the debugger has been initialized. Furthermore, a previous change prevented running the application until after the "configurationDone" request is received. Therefore, with this change, there is now a well defined window (between sending the "initialized" event and receiving the "configurationDone" request), for breakpoint configuration to occur.
brownts
force-pushed
the
bugfix/breakpoint_race_condition
branch
from
27829e8
to
12f9a7a
Compare
Fixes #307.
The previous implementation worked under the assumption that
breakpoint requests would be received from the client prior to the
debugger having completed it's initialization. However, if the
debugger completed initialization prior to having received breakpoint
requests from the client, those breakpoints requests would be
ignored (i.e., there was a race condition). Additionally, the
previous implementation "buffered" the receipt of the breakpoint
requests and would only process them after the debugger initialization
had completed (i.e., "debug-ready").
This change delays the sending of the "initialized" event back to the
client (until the debugger truly has completed its initialization).
As a result, there is no longer a need to buffer the breakpoint
requests. Breakpoints requests are now forced to be received after
the debugger has been initialized. Furthermore, a previous change
prevented running the application until after the "configurationDone"
request is received. Therefore, with this change, there is now a well
defined window (between sending the "initialized" event and receiving
the "configurationDone" request), for breakpoint configuration to
occur.