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

[Debug] Add the possibility to know when the debug widget has been cleared #8639

Closed
federicobozzini opened this issue Oct 15, 2020 · 5 comments · Fixed by #8645
Closed

[Debug] Add the possibility to know when the debug widget has been cleared #8639

federicobozzini opened this issue Oct 15, 2020 · 5 comments · Fixed by #8645
Labels
debug issues that related to debug functionality proposal feature proposals (potential future features)

Comments

@federicobozzini
Copy link
Contributor

Feature Description:

When a debug session is terminated, the debug widget is cleared.

This operation requires the debug widget to be visible. This means that at the moment it's not possible to perform some operations (like switching to a different panel) when a debug session is terminated without generating a race condition.

It would be helpful to have some way to update the debug widget to clear even when not visible or fire an event when the clearing is completed.

There are different ways to tackle this. To me it looks like the simplest way would be to add a property (false by default) on TreeWidget to make it always updatable. What do you think?

@vince-fugnitto
Copy link
Member

@federicobozzini would it not be easiest to rely and listen to the debug events?

readonly onDidDestroyDebugSession: Event<DebugSession> = this.onDidDestroyDebugSessionEmitter.event;

readonly onDidChangeActiveDebugSession: Event<DidChangeActiveDebugSession> = this.onDidChangeActiveDebugSessionEmitter.event;

export interface DidChangeActiveDebugSession {
previous: DebugSession | undefined
current: DebugSession | undefined
}

I'm not sure what problem is solved by knowing when the debug-widget is cleared, or has no debug sessions that the service would not tell you.

@vince-fugnitto vince-fugnitto added 🤔 needs more info issues that require more info from the author debug issues that related to debug functionality proposal feature proposals (potential future features) labels Oct 15, 2020
@federicobozzini
Copy link
Contributor Author

Let's say that you want to change the active panel when the debug session is terminated.

Of course you could use the events you just linked. The problem is that the debug information shown during the debug session (threads, breakpoints, watched variables, ...) is not cleared if the debug widget is not visible because the active panel replaces it too soon. Those events are fired before the debug information is cleared and this generates a race condition.

@vince-fugnitto
Copy link
Member

Of course you could use the events you just linked. The problem is that the debug information shown during the debug session (threads, breakpoints, watched variables, ...) is not cleared if the debug widget is not visible because the active panel replaces it too soon. Those events are fired before the debug information is cleared and this generates a race condition.

If I understand correctly, the issue is that you want to change the active panel when a debug session is terminated, but this results in a race condition since you rely on the widget and not the debug events to determine when to switch panels? If its the case I'd argue not to rely on the debug-widget clearing but the actual debug session events I mentioned earlier.

Else, I'd argue that the DebugViewModel should be the source from which you determine when to change the active panel. The model can inform you when there is no active session, and only then would you want to switch panels.

Of course I might be misunderstanding the use-case completely.

@federicobozzini
Copy link
Contributor Author

If I understand correctly, the issue is that you want to change the active panel when a debug session is terminated, but this results in a race condition since you rely on the widget and not the debug events to determine when to switch panels?

No this is not correct, sorry for the confusion. The race condition is exactly what I get if I try to use the debug events to switch the active panel. This happens because, in this case, both the active panel switch and the clearing of the debug widget happen as a result of the debug event getting fired. If the panel is switched before the debug widget is cleared, the clearing doesn't happen at all, because the debug widget needs to be visible.

Given that, I think the two possible ways to avoid this situation is either to switch the active panel after the debug widget is cleared or just allow the debug widget to refresh when it's not visible.

I'm not sure if DebugViewModel might help here. I think the model is updated (and its events are fired) before the debug widget is cleared, so switching panel after that might still make the race condition possible.

@vince-fugnitto
Copy link
Member

No this is not correct, sorry for the confusion.

Thank you for clarifying!
I believe whichever approach to resolve the problem would be fine, we can review properly during a pull-request.
It may be worthwhile to ask the original author of the debug-view what the best approach may be if we are still unsure.

@vince-fugnitto vince-fugnitto removed the 🤔 needs more info issues that require more info from the author label Oct 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debug issues that related to debug functionality proposal feature proposals (potential future features)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants