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

Add support for "lifecycleManagedByParent" #51

Closed

Conversation

tsmaeder
Copy link
Collaborator

@tsmaeder tsmaeder commented Oct 6, 2022

What it does

Adds support for the lifecycleSupportedByParent flag on debug session. Fixes eclipse-theia#11511

How to test

Contributed on behalf of ST Microelectronics

Review checklist

Reminder for reviewers

Forwards lifecycle events to parent session where applicable and changes
the behavior of compound sessions to behave same way.

Contributed on behalf of ST Microelectronics

Signed-off-by: Thomas Mäder <[email protected]>
@sdirix sdirix changed the base branch from master to 0.6.1 October 7, 2022 07:22
@sdirix sdirix changed the base branch from 0.6.1 to master October 7, 2022 07:22
@planger planger self-requested a review October 7, 2022 08:41
Copy link

@planger planger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks! Code looks good to me.

What I've successfully tested is below.
Additionally to that, it may be worth creating a minimal VS Code extension based on mock-debug and compare that with how VS Code behaves for the same minimal extension. What do you think @tsmaeder?

What I've tested

Setup

  • Launch Electron Theia with ms-vscode.js-debug-1.61.0.vsix
  • Out of Electron Theia start launch configs for browser Theia
  • Scenarios below in comparison with VS Code

Launch Config: Browser backend only (not compound)

Theia

  • stop -> stops all
  • restart -> restarts all

VS Code

  • stop -> stops all
  • restart -> restarts all

Launch Config: Compound Launch Browser backend and frontend (with stopAll = false)

Theia

  • stop -> stops parent process but not browser
  • restart -> restarts parent process but not browser

VS Code

  • stop -> stops parent process but not browser
  • restart -> restarts parent process but not browser

Launch Config: Compound Launch Browser backend and frontend (with stopAll = true)

Theia

  • stop -> stops parent process AND browser
  • restart -> restarts parent process but not browser

VS Code

  • stop -> stops parent process AND browser
  • restart -> restarts parent process but not browser

@tsmaeder
Copy link
Collaborator Author

tsmaeder commented Oct 7, 2022

Additionally to that, it may be worth creating a minimal VS Code extension based on mock-debug and compare that with how VS Code behaves for the same minimal extension.

I think the only scenario we can't cover with the js-debug extension is the one where there is a process tree where some have the flag lifecycleManagedByParent is set for some, but not others: we could check that the propagation of the lifecycle events stops at a process tree root where the flag is not set. Not sure whether that is worth the effort, though.

One bunch of scenarios I didn't see in your testing is when the processes are not terminated by hand, but terminate themselves (for example, killing a subprocess by hand). The lifecycle events should not propagate in this case (if I read the doc correctly).

@planger
Copy link

planger commented Oct 7, 2022

One bunch of scenarios I didn't see in your testing is when the processes are not terminated by hand, but terminate themselves (for example, killing a subprocess by hand). The lifecycle events should not propagate in this case (if I read the doc correctly).

Right, good point! I've tested that now too.

Theia

  • end plugin-host process (with stop all) -> Theia backend process keeps running
  • end plugin-host process (without stop all) -> Theia backend process keeps running

VS Code

  • end plugin-host process (with stop all) -> Theia backend process keeps running
  • end plugin-host process (without stop all) -> Theia backend process keeps running

@planger planger self-requested a review October 7, 2022 18:22
Copy link

@planger planger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the nice work!

Signed-off-by: Thomas Mäder <[email protected]>
@sdirix
Copy link
Member

sdirix commented Nov 16, 2022

Merged upstream with eclipse-theia#11751

@sdirix sdirix closed this Nov 16, 2022
@sgraband sgraband mentioned this pull request Jul 31, 2024
1 task
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.

[vscode] Support debug sessions being managed by a parent session
3 participants