eclipse / lsp4e

Language Server Protocol support in Eclipse IDE
Eclipse Public License 2.0
57 stars 54 forks source link

DSPThread: cannot be suspended if program is terminated #1033

Closed agarciadom closed 1 week ago

agarciadom commented 1 week ago

We have started to experiment with replacing our custom debuggers in the Eclipse Epsilon project with LSP4E-based debuggers, and we've noticed one odd issue when we stop at a breakpoint and then resume and let the program stop. This pull request proposes a workaround for the issue for the time being - the deeper issue seems to be how to handle the case where a continue request resumes all threads.

Scenario

In one of our test programs, we have a main thread T1 start, then another thread T2 starts and stops at a breakpoint. In the Epsilon debug adapter, we stop all threads when we hit a breakpoint, so our stopped event provides both the specific thread that hit the breakpoint, and also calls setAllThreadsStopped(true). Our program stops as expected, like this:

image

I press F8 to continue (which continues all threads: we indicate this in our response to the continue request by calling setAllThreadsContinued(true)), and hit the next breakpoint as usual:

image

I press F8 once more, and the program finishes successfully, but I'm left with this odd state:

image

From what I gathered by debugging, it looks like resuming the thread did not clear the isSuspended flag from the other thread, despite the response from the debug adapter that all threads had been continued. I saw that we got to this part of DSPThread just fine, and the resume event was fired:

getDebugProtocolServer().continue_(arguments).thenAccept(response -> {
    if (response == null || response.getAllThreadsContinued() == null || response.getAllThreadsContinued()) {
        // turns out everything was resumed, so need to update other threads too
        getDebugTarget().fireResumeEvent(DebugEvent.CLIENT_REQUEST);
    }
}).exceptionally(this::handleExceptionalResume);

However, I did not notice the continued() method being called for the other thread, despite this response. We send out thread start and exit events, so if those events are processed in time before the terminated event then the threads are removed by the time the program exits, but it's not always the case.

Workaround

I propose changing the isSuspended() method in DSPThread so it won't return true if the program is terminated. This fixes the issues around showing terminated threads in a program as paused or as available to be resumed. This small change results in the desired output:

image

mickaelistria commented 1 week ago

Thanks for this good patch. I'd like to merge it ASAP. Can you please just add a commit that bumps the version by +0.0.1 for the modified bundle? As it's the first change since it's last released, this is required to bump the version before creating a new build.

agarciadom commented 1 week ago

Sure! I have pushed another commit that bumps up lsp4e.debug from 0.15.8 to 0.15.9.

rubenporras commented 1 week ago

thanks