Vscode: VS Code holds onto parent console when launched via command line

[Tyriar]

Repro:

1. Open cmd.exe
2. Run code
3. Switch back to cmd.exe
4. exit

Repro's in stable and insiderrs and does not repro in Atom which is also on Electron 2.

Advice from Windows console team:

I believe VSCode is reconnecting to its parent hosting console for debugging purposes.

Starting with @joaomoreno who might know who to send this to, not sure who the real owner is though.

[DHowett-MSFT]

FWIW, The console is working as-designed. Since there is a connected client application, we won't exit because we don't want to sever its standard input/output/error handles (or the console handle it has allocated.)

[joaomoreno]

@Tyriar We have had some issues like this in the past. I do not know nearly enough about Electron, Windows or Windows console infrastructure to make an informed decision. If you have some ideas, solution, let me know.

[Tyriar]

@DHowett-MSFT we don't call AttachConsole anywhere AFAIK, is there any other way to attach to the console? Doing a search of the whole codebase+deps the only place AllocConsole, FreeConsole and AttachConsole are used is within node-pty/winpty.

[DHowett-MSFT]

We're definitely seeing Code (electron.exe, really) calling AttachConsole during startup:

0775f224 75c8a500 ntdll!NtCreateFile+0xc
0775f7fc 75d373db KERNELBASE!ConsoleCreateConnectionObject+0x158
0775fa44 75d3717b KERNELBASE!ConsoleAttachOrAllocate+0x91
0775fa70 0066755a KERNELBASE!AttachConsole+0x3b
WARNING: Stack unwind information not available. Following frames may be wrong.
0775fa7c 004e26f9 Code+0x21755a
0775fb04 02584bc8 Code+0x926f9

Unfortunately, I don't have symbols for Code/electron, so I can't point out the originating line.

(This backtrace was generated by setting a breakpoint on _conhost_'s connection request handler and then investigating the originating process to see where it was stuck.)

[DHowett-MSFT]

I am, however, convinced that this line is the cause. I can't find RouteStdioToConsole anywhere but Chromium's source, but it looks like that is what's calling AttachConsole.

[Tyriar]

@DHowett-MSFT I just spotted that as well, maybe the Electron build we're on isn't official. @bpasero our Electron builds may be being built without OFFICIAL_BUILD set:

https://cs.chromium.org/chromium/src/mash/runner/main.cc?type=cs&q=RouteStdioToConsole&sq=package:chromium&g=0&l=42

This likely has performance implications as well if this is the case.

[bpasero]

@Tyriar I got some homework for you:

• find out if this is true for our Microsoft build of Electron and/or the GH release to see if this is an issue with our internal build or a general issue with the GH build
• if this is an issue with GH build, report it to the Electron repo (https://github.com/atom/electron)
• if this is an issue with the MS build, I can forward this to the team that produces the build

[bpasero]

@Tyriar it looks like only starting with Electron 4.0.x we will benefit from this flag being set.

//cc @deepak1556

[deepak1556]

Electron does build with chromium OFFICIAL_BUILD flag since 3.x. The source of the issue is the code from electron https://github.com/electron/electron/blob/master/atom/app/atom_main.cc#L132 that attaches to the console mentioned by @DHowett-MSFT .

[Tyriar]

OK I'm a little confused now, doesn't code.cmd do the following:

1. Launch the CLI with set ELECTRON_RUN_AS_NODE=1 (we want the CLI to attach)
2. The CLI deletes ELECTRON_RUN_AS_NODE
3. The CLI launches the executable (not run as node should not attach)
4. The CLI exits which returns to the prompt (nothing should be attached)

Even adding this line to code.cmd set ELECTRON_NO_ATTACH_CONSOLE=1 will repro the same problem which should certainly not hit that Electron code path, so maybe it is an OFFICIAL_BUILD issue? I'll test the open source build which will pull official Electron releases, that should hopefully verify.

[Tyriar]

@deepak1556 does main.cc not get run in Electron as it's replaced by atom_main.cc?

[deepak1556]

atom_main.cc is the entry point to chromium's main which is invoked by content::ContentMain().

@Tyriar in your test case can you make sure the env variable ELECTRON_NO_ATTACH_CONSOLE is sent by CLI that launches the executable in step 3.

[Tyriar]

@deepak1556 I checked the renderer's process.env and it was there, so it seems to have been set all the way through. It's also set explicitly here in the CLI before launching the main process:

https://github.com/Microsoft/vscode/blob/ac1812d748463ca1cb1e6777e7029134f4b56f2f/src/vs/code/node/cli.ts#L117-L124

There's also some code here that deals ELECTRON_NO_ATTACH_CONSOLE inside the main process when it's setting up its env:

https://github.com/Microsoft/vscode/blob/ac1812d748463ca1cb1e6777e7029134f4b56f2f/src/vs/code/node/shellEnv.ts#L14

But it looks like it just carries it over:

https://github.com/Microsoft/vscode/blob/ac1812d748463ca1cb1e6777e7029134f4b56f2f/src/vs/code/node/shellEnv.ts#L52-L61

[bpasero]

Adding @joaomoreno who seems to have added ELECTRON_NO_ATTACH_CONSOLE via https://github.com/Microsoft/vscode/commit/cd779ff1c7e45336b728f6dcc8aa585f4c1b492d

I am still not fully understanding the issue here and if the OFFICIAL_BUILD flag is missing or not.

[Tyriar]

@deepak1556 did some investigation and it appears that the main process isn't being called with ELECTRON_NO_ATTACH_CONSOLE here:

https://github.com/Microsoft/vscode/blob/ac1812d748463ca1cb1e6777e7029134f4b56f2f/src/vs/code/node/cli.ts#L359

I'll fix this if I have time, otherwise feel free to pick it up @joaomoreno.

[Tyriar]

@deepak1556 you said this in Slack:

I logged the process.env in electron-main, there is no ELECTRON_NO_ATTACH_CONSOLE

How did you log this exactly?

I logged options.env here:

https://github.com/Microsoft/vscode/blob/9301aefe0f5b8c2505421ee57721e5bcfde2e117/src/vs/code/node/cli.ts#L359

And it definitely showed up:

[deepak1556]

The cli options does pass that env variable to child_process.spawn, but if it was available for main process of the electron app then I expected it to show up in https://github.com/Microsoft/vscode/blob/master/src/vs/code/electron-main/main.ts and it didn't.

[the-ress]

This is caused by base::RouteStdioToConsole opening four new handles to console. Then they're inherited into the spawned process.

I'll create a PR with the fix in the electron repo soon.

Edit: related comment in #33256

[Tyriar]

Upstream PR for this: https://github.com/electron/electron/pull/18698

[jpelton-stroud]

I'll create a PR with the fix in the electron repo soon.

Any update on this? It's a minor but extremely annoying issue.

Edit: sorry, just now noticed the linked PR; really appreciate you all working on it!

Edit2: the PR appears to be stalled :( anyone know of a workaround to detach it after the fact?

[jtnord]

Any update on this? It's a minor but extremely annoying issue.

not really minor as your shell (cmd.exe) becomes almost useless as it has debug logs spewed to it which makes reading what you should have there almost impossible.

Note: there are people saying "use code.bat" not "code.exe" however - that workaround does not work when you use start some_file_with_type_registered_to_code. for example if you associate xml files with code via the windows explorer (always open with) and then from a cmd.exe shell run start test.xml which is pretty basic windows behaviour you get the debug messages in the console. You can not change the association to code.bat as that spawns a new console window when you open the file from explorer

Edit: sorry, just now noticed the linked PR; really appreciate you all working on it!

the upstream PR has been closed and so appears stalled for real this time.