-
Notifications
You must be signed in to change notification settings - Fork 29.9k
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
Reattached to the wrong terminal #133542
Reattached to the wrong terminal #133542
Comments
Just so you know, this happens pretty often. I just reloaded a window and got some random terminal from another window. Let me know if I can collect any other info |
I was able to repro once out of many attempts. I wonder if there's a command to reload all windows to simulate what happens on update? Maybe then I'd see it? |
@meganrogge exiting the application (cmd+q) and reopening it is essentially what happens on update |
Happened again, accidentally ran a destructive command in the wrong directory :( cc @Tyriar @meganrogge is there any logging or something I can enable to help you get to the bottom of this? |
I spent awhile investigating. I don't understand what could be going on here because we store and reconnect to terminals according to the workspaceId. @Tyriar said that he thinks it's a race condition. Setting log level to trace and toggling the shared process would help when it reproes |
Today I had something new happen:
No other windows were reloaded. No windows were closed. Is there a way I can disable terminal reconnect for local windows? I've also run destructive commands when this happens and I don't want to risk it. |
Yep, it's not working atm for remote terminals, so:
|
Hello everyone, there is any plan to try to fix it in the nearest milestone? |
This code that we added for transferring terminals via dnd between windows looks suspicious vscode/src/vs/workbench/contrib/terminal/browser/terminalService.ts Lines 274 to 292 in d982c14
|
Has anyone seen this in awhile? I cannot reproduce 😢 |
I don't remember seeing it recently |
Okay pls reopen if you see it again and I'll be happy to debug with anyone that can reproduce it |
Sorry, I cursed myself because this happened again today but I probably won't have consistent repro steps. I reloaded the window with my vscode workspace and got a terminal from this extension sample which is also open in another window, although the terminal panel had not been made visible in that other window's session yet. I reloaded the window several more times and always got the right terminal after that. |
I haven't seen this in a long time. Not sure whether there is anything that can be done to verify it. |
I just ran into this now:
Pty logs from Window 2:
|
@alexr00 thanks for the info! Looking into it |
I spent a few hours looking into this, here are my findings:
Current plan is to wait for #186645 to merge and await a repro with these logs present, then analyze them. I'll also likely be continuing to fiddle with terminal reconnection in debt week to improve perf further and reduce redundancies. |
I'll keep an eye out for this and try to maintain a setup similar that one that I just saw it with, but I have to say, this has gotten so much better. I almost never see it. |
I tried to repeat my setup before updating, and it did happen again.
After update:
Logs:
|
Thanks @alexr00, it seems there's some inconsistency with the layout info and the revived procs, so I suspect this silently error swallowing now: vscode/src/vs/workbench/contrib/terminal/electron-sandbox/localTerminalBackend.ts Lines 301 to 303 in eafde5a
Added logs for that here: #186732, I'll merge for July so you might not be able to test for a week but when you do please also grab the Terminal output log as this is renderer side. |
Cross post #186922 (comment)
|
With @meganrogge's logs in #186922, I was able to reproduce the problem! The repro is very complicated, it is essentially trying to get into a state where 2+ windows have "old pty ids" that conflict with one another, and then increasing the latency of the pty host in order to force the race condition to show itself.
The terminal with The fix for this is simple, just add workspace ID as part of the key of the old id, so |
I just reloaded a window with my
vscode-jupyter
repo open, and it attached to a terminal which is already open in myvscode
repo, so now this terminal is open in two windows:The right window is the window that originally owned this terminal
One thing about this terminal is that its cwd is some other folder outside of both workspaces, if that matters.
The text was updated successfully, but these errors were encountered: