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

ExtensionHost debugging: debug action window disappears briefly #43516

Closed
weinand opened this issue Feb 12, 2018 · 18 comments
Closed

ExtensionHost debugging: debug action window disappears briefly #43516

weinand opened this issue Feb 12, 2018 · 18 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug debug Debug viewlet, configurations, breakpoints, adapter issues verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@weinand
Copy link
Contributor

weinand commented Feb 12, 2018

  • VS Code Stable or Insiders
  • Windows 10

Steps:

  • create hello world extension
  • open in VS Code
  • press F5

Observe:
Debug action bar disappears and comes back.

It looks like an additional short-living session is created on startup.

I do not see this behaviour on macOS and linux.

@weinand weinand added the debug Debug viewlet, configurations, breakpoints, adapter issues label Feb 12, 2018
@isidorn
Copy link
Contributor

isidorn commented Feb 12, 2018

This is happening due to how we are connecting to the extension host.
This code path is very delicate since it handles both start, reload, quit, restart. Due to that pushing to backlog and will adress only if there is a bigger brekage.

@isidorn isidorn added the bug Issue identified by VS Code Team member as probable bug label Feb 12, 2018
@isidorn isidorn added this to the Backlog milestone Feb 12, 2018
@weinand weinand modified the milestones: Backlog, On Deck Feb 12, 2018
@weinand weinand self-assigned this Feb 12, 2018
@weinand
Copy link
Contributor Author

weinand commented Feb 12, 2018

We need to understand this and then we should really make this robust enough so that it does not result in this behaviour on just one platform.

We can investigate this together...

@weinand
Copy link
Contributor Author

weinand commented Mar 14, 2018

This happens now on all platforms.

@weinand
Copy link
Contributor Author

weinand commented Mar 14, 2018

With the introduction of the "postDebugTask" the "bigger breakage" has now arrived: with this bug "postDebugTask" is no longer symmetric to "preLauchTask".

@weinand weinand modified the milestones: March 2018, April 2018 Mar 26, 2018
@weinand
Copy link
Contributor Author

weinand commented Apr 20, 2018

@isidorn what is the event that makes you turn down the debug session?

@isidorn
Copy link
Contributor

isidorn commented Apr 20, 2018

@weinand I will now write down the exact ordering of events and what is going on so we can analyize on what is best to be done here.

@isidorn
Copy link
Contributor

isidorn commented Apr 20, 2018

Extension launched

  1. Process is created
  2. I get a broadcast Attach. Currently I handle this by ending a process and starting a new one thus the flash. Code pointer. I remember I am doing this as a workaround due to some other timing issues.
  3. Process is ended
  4. Process is created

Extension restart from first window

  1. Adapter exits -> thus process ends
  2. I get a broadcast Attach, thus I try to end the process (does not exist) and create a new one
  3. New process is created

Extension restart by pressing reload in the second window

  1. broadcast Attach -> thus I stop the process and start a new one
  2. End the process
  3. Starting the new one

Notice that in this case I am not getting any event that the debugger has exited and I believe that is the reason why I choose to always end and start on broadcast Attach

Extension is ended from first window

  1. Adapter exits -> and process is ended

Extension is ended by clicking close on the second window

  1. Adapter exits -> and process is ended

@isidorn
Copy link
Contributor

isidorn commented Apr 20, 2018

We also need to be aware of that timing on different machines can be different. I remember that sometimes I would first get the broadcast and only then the adapter exit event and this would screw up my logic, however the current approach is also not error prone to this it seems.

So from this analysis the main issue is the reload in the second window which only sends the Attach. Ideally we would change this to not be different from the extension restart from first window, however I believe this is not possible.
I could investigate if it is possible to send some special flag in the broadcast such that we at least we know that it is this "special" case

@weinand
Copy link
Contributor Author

weinand commented Apr 20, 2018

The issue at hand is only item 2. under "Extension launched":

You kill the process that you've just created because you do not know that the attach is not a re-attach. Does it help if you introduce a flag ("somewhere") that tells you whether this is the first attach (which does not require to kill anything).

If you cannot find a place (i.e. object) where to store that flag, could you look at the events you receive to find out whether it is a restart?

@weinand weinand removed their assignment Apr 23, 2018
@weinand weinand added verification-needed Verification of issue is requested verified Verification succeeded labels Apr 23, 2018
@weinand
Copy link
Contributor Author

weinand commented Apr 23, 2018

While verifying this I noticed another issue: #48442

@weinand weinand modified the milestones: May 2018, April 2018 Apr 24, 2018
@roblourens
Copy link
Member

Nice diagram, we need a couple poster-sized prints!

@weinand
Copy link
Contributor Author

weinand commented Apr 25, 2018

The fix worked for me when running out of source.
But in the real build it does not work.
So the issue seems to have a timing aspect as well.

Moving back to Isidor.

@weinand weinand assigned isidorn and unassigned roblourens Apr 25, 2018
@weinand weinand modified the milestones: April 2018, May 2018 Apr 25, 2018
@isidorn
Copy link
Contributor

isidorn commented Apr 25, 2018

@weinand thanks for investigating. I will look into this

@weinand weinand modified the milestones: May 2018, April 2018 Apr 27, 2018
@weinand
Copy link
Contributor Author

weinand commented Apr 27, 2018

Thanks to @isidorn's perseverance we now understand why the new fix in node-debug2 was working for me: I still had the original fix for this issue in my workspace. And the new fix was relying on the old fix (but I wasn't aware of this).
So when I reverted the original fix, my new fix stopped working.
So reverting the reverted fix fixed everything...

@weinand weinand added verification-needed Verification of issue is requested verified Verification succeeded labels Apr 27, 2018
isidorn added a commit that referenced this issue Apr 30, 2018
This reverts commit c7218a8.
@isidorn
Copy link
Contributor

isidorn commented Apr 30, 2018

The fix resulted in #48955, thus reverting and assigning to May.

@isidorn isidorn reopened this Apr 30, 2018
@isidorn isidorn modified the milestones: April 2018, May 2018 Apr 30, 2018
@isidorn isidorn removed the verified Verification succeeded label Apr 30, 2018
isidorn added a commit that referenced this issue Apr 30, 2018
This reverts commit c7218a8.
@isidorn isidorn closed this as completed in 9eef194 May 4, 2018
@isidorn
Copy link
Contributor

isidorn commented May 4, 2018

I have fixed this by not restarting the whole DebugSession, but only ending the rawSession and starting a new session. And since DebugSession now survives a reaatach there are no more flashes.
I have verified this works on my mac. To verify:

Debug an extension and

  1. Reload in eh window keeps the debug session connected
  2. Close in eh windows ends the debug session
  3. Restart in first window nicely reloads the eh window
  4. Stop in first window nicely closes the eh window

@RMacfarlane RMacfarlane added the verified Verification succeeded label May 30, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators Jun 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug debug Debug viewlet, configurations, breakpoints, adapter issues verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

4 participants