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 telemetry event when the pty host becomes unresponsive #120185

Closed
Tyriar opened this issue Mar 30, 2021 · 4 comments
Closed

Add telemetry event when the pty host becomes unresponsive #120185

Tyriar opened this issue Mar 30, 2021 · 4 comments
Assignees
Labels
debt Code quality issues insiders-released Patch has been released in VS Code Insiders terminal General terminal issues that don't fall under another label
Milestone

Comments

@Tyriar
Copy link
Member

Tyriar commented Mar 30, 2021

@Tyriar Tyriar added debt Code quality issues terminal General terminal issues that don't fall under another label labels Mar 30, 2021
@Tyriar Tyriar added this to the April 2021 milestone Mar 30, 2021
@Tyriar Tyriar self-assigned this Mar 30, 2021
@Tyriar Tyriar modified the milestones: April 2021, May 2021 Apr 21, 2021
@bpasero
Copy link
Member

bpasero commented Apr 27, 2021

Running a quick search over how many shared processes report "Unresponsive" (an electron event), I see around 6000 clients hitting this event for 1.55.2. We still have no clear understanding how the shared process can become unresponsive overall, but we need to understand this going forward if the shared process is the home of the extension host and file services.

Having a better understanding of what happens in the terminal host in this case would be helpful 👍

@Tyriar
Copy link
Member Author

Tyriar commented Apr 27, 2021

I have a repro and have tried to mitigate it but it didn't end up working: #121336

Was working on this yesterday but still having problems. The root cause I fixed months ago but am blocked by workers not working only in the product build due to asar which I've been chatting to @deepak1556 about. It would be lovely to get this fixed for April but am waiting for @deepak1556 to free up as I've spent way too long on this with no success.

And yes I'm sick of adding mitigation on top of mitigation for a bug that I've already fixed ☹️

@Tyriar
Copy link
Member Author

Tyriar commented Apr 27, 2021

Running a quick search over how many shared processes report "Unresponsive" (an electron event)

Also, are you sure this is related? Does this event fire when a child process of a window is unresponsive? The pty host doesn't take down the shared process.

@Tyriar Tyriar modified the milestones: May 2021, April 2021 Apr 27, 2021
@Tyriar Tyriar closed this as completed in a6591d4 Apr 27, 2021
@bpasero
Copy link
Member

bpasero commented Apr 27, 2021

@Tyriar yeah sorry, I should have made more clear: I cannot tell the unresponsiveness stems from the terminals host, just a speculation. I have now added more telemetry to learn what can cause this. So far we have tracked if the shared process window has been made visible by the user (rather unlikely) but now more importantly I track if the VSCode window is shutting down when this happens. According to Deepak, unresponsiveness can also fire when a shutdown takes more than 5s which maybe is the case for shared process.

telemetryService.publicLog2<SharedProcessErrorEvent, SharedProcessErrorClassification>('sharedprocesserror', {
type,
reason: typeof details !== 'string' ? details?.reason : undefined,
visible: sharedProcess.isVisible(),
shuttingdown: willShutdown
});

@github-actions github-actions bot locked and limited conversation to collaborators Jun 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debt Code quality issues insiders-released Patch has been released in VS Code Insiders terminal General terminal issues that don't fall under another label
Projects
None yet
Development

No branches or pull requests

4 participants
@bpasero @Tyriar @meganrogge and others