terminal: ClosePseudoConsole API hanging

Environment

Windows build number: Microsoft Windows [Version 10.0.18362.175]

Any other software? Visual Studio Code Insiders

Steps to reproduce

  1. Launch VS Code Insiders.
  2. Ensure using conpty with setting: terminal.integrated.windowsEnableConpty
  3. Open a terminal with Ctrl+`
  4. Close the terminal with the trash can icon
  5. Repeat 3 and 4 until the window hangs. It takes around 10 retries on my machine.

Expected behavior

No hanging.

Actual behavior

The window hangs. Using windbg, I see the following:

conpty_stack conpty_locals conpty_leftovers

Notice the many leftover conhosts in the task manager as well as the hang in ClosePseudoConsole.

Here is the link to the code in node-pty calling into the PseudoConsole API. https://github.com/microsoft/node-pty/blob/04445ed76f90b4f56a190982ea2d4fcdd22a0ee7/src/win/conpty.cc#L429

/cc @daimms

About this issue

  • Original URL
  • State: closed
  • Created 5 years ago
  • Reactions: 16
  • Comments: 58 (40 by maintainers)

Commits related to this issue

Most upvoted comments

@miniksa hopefully it can get sorted, it currently causes all kinds of hard locks in Visual Studio Code at the moment in any terminal. To reproduce, just type ‘a’ and kill the process. Capture

Quite frustrating 😕

Probably tomorrow’s insiders, follow https://github.com/microsoft/vscode/pull/116185 for updates

As mentioned earlier in this thread:

to be 100% clear, VS Code can fix this on the client end by properly threading the handles received from the ConPTY. The hang is because all of the ConPTY handles are being serviced on the same thread in VS Code. To resolve this condition, the output and input handles should be managed on separate threads/queues than the one that is managing the session.

@christianparpart could you do something like that?

I am currently actively looking into finally working around this ConPTY behavior.

Honestly, if your above quoted workaround is the only way to work around this, I’d rather drop Windows support (sadly) or just wait until maybe or maybe not eventually ConPTY can handle with read/write operations in one thread. The reasoning is not that I want to be stubborn but rather that ConPTY (windows) is the only platform that requires this architectural change just to not freeze at shutdown.

I’m trying it though, but it seems like not worth it for now. 😃

UPDATE: Okay, I’ve actually forgotten my own source code, and I remember why I did already split up PTY reads from writes (because ConPTY does not support non-blocking I/O).

My problem was, that if the connected process behind ConPTY terminated, the already started ConPTY’s read() call did not return and does hang forever. This is still the case with latest software updates, and I actually do consider this a bad (or buggy?) behaviour compared to the other PTY implementations cancelling any active I/O operation when the other end disconnects. I did add a process exit watcher that does explicitly close the terminal’s end of the PTY, and now the read-call is not hanging anymore.

Thanks 😃

Is there any movement on this issue? It causes hangs for many vscode users.

VS Code update: Electron isn’t going to support worker_threads in the renderer thread, so the fix is blocked on moving where we host node-pty/conpty to a real node process: https://github.com/microsoft/node-pty/pull/415#issuecomment-688870219

OK. We’ll see. Thanks for the pointer to the code. We’ll have to see where/when we can find resources for that and for this issue.

I just hit the hang when closing a regular terminal (not a task) which has PSEUDOCONSOLE_INHERIT_CURSOR set to 0. Killing conhost freed up the UI again.

Next time you hit the hang or any other hang like this, can you please take a process dump of the supposedly hung conhost so I can check the stacks?

If you’re hitting a similar situation without the flag set, then you’re getting hung in a different situation which I’m not necessarily working on fixing right now in this issue.

@miniksa installer/zip at the top: https://code.visualstudio.com/updates/v1_36, you can set "update.mode": "none" to disable automatic updates

This is because the ConPTY is being started with CreatePseudoConsole() with the flag PSEUDOCONSOLE_INHERIT_CURSOR. There’s an indefinite wait for the calling terminal to respond with the cursor position before the startup lock is freed for other threads to use. A different thread gets the shutdown message and is attempting to acquire the same lock to clean things up before exiting.

It looks like if the PTY starts up fast enough so that the shutdown message comes after the cursor is inherited OR if the shutdown happens before the PTY starts asking for the cursor inherit, then everything is good.

Given this is a race condition, I can’t really blame it on the caller holding it wrong. I’ll work to resolve it such that a shutdown is a valid way of halting the request for the cursor position while it is starting up.