Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DSPThread: cannot be suspended if program is terminated #1033

Merged
merged 2 commits into from
Jul 3, 2024

Conversation

agarciadom
Copy link
Contributor

@agarciadom agarciadom commented Jul 1, 2024

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
Copy link
Contributor

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
Copy link
Contributor Author

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

@rubenporras
Copy link
Contributor

thanks

@rubenporras rubenporras merged commit 2ed013b into eclipse:main Jul 3, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants