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

Windows Terminal leaks a file handle to the starting directory #10849

Closed
zomgrolf opened this issue Aug 2, 2021 · 20 comments
Closed

Windows Terminal leaks a file handle to the starting directory #10849

zomgrolf opened this issue Aug 2, 2021 · 20 comments
Labels
Area-TerminalConnection Issues pertaining to the terminal<->backend connection interface Help Wanted We encourage anyone to jump in on these. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. Resolution-Duplicate There's another issue on the tracker that's pretty much the same thing.

Comments

@zomgrolf
Copy link

zomgrolf commented Aug 2, 2021

Windows Terminal version (or Windows build number)

1.9.1942.0 (Windows version: 10.0.19043.1110)

Other Software

No response

Steps to reproduce

  1. Create an empty directory, e.g. c:\temp\rmdir_test
  2. Open explorer.exe and navigate to the newly created directory.
  3. In the address bar, type wt -d . to launch a new instance of Windows Terminal with Command Prompt as the default profile.
  4. Close the explorer window.
  5. In the command prompt type cd .. to go the the directory above (c:\temp).
  6. Type rmdir rmdir_test to attempt to delete the empty rmdir_test directory

Expected Behavior

I was expecting to be able to delete the rmdir_test directory without having to close the terminal window.
If I repeat the above steps using conhost instead of Windows Terminal (i.e. typing cmd.exe in the address bar instead of wt -d .), I can delete that directory just fine, with the command prompt still open.

Actual Behavior

I get the following error: The process cannot access the file because it is being used by another process.

If I look for the handle in Resource Monitor, I get this:
resmon

It seems that the Terminal retains that file handle for as long as the process is alive, even if I navigate away from the starting directory.

Seems to be similar to #9664, which was marked as resolved and closed, although I don't really see any solutions in there that would be applicable in my case...

@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Aug 2, 2021
@vefatica
Copy link

vefatica commented Aug 2, 2021

It's still the current working directory of windowsterminal.exe and openconsole.exe.

@zomgrolf
Copy link
Author

zomgrolf commented Aug 2, 2021

Possibly, so?

@vefatica
Copy link

vefatica commented Aug 2, 2021

A process always has an open handle to its current working directory.

@zomgrolf
Copy link
Author

zomgrolf commented Aug 2, 2021

And the current working directory stays the same forever because...?

@eryksun
Copy link

eryksun commented Aug 2, 2021

A process keeps its working directory open without shared delete access, which prevents the directory from being deleted or renamed until the directory is closed (e.g. the working directory changes or the process terminates). Notably, the base API uses the handle for the working directory to open relative file paths (see NTAPI OBJECT_ATTRIBUTES RootDirectory), which is similar to openat() in POSIX systems.

And the current working directory stays the same forever because...?

Because "WindowsTerminal.exe" and "OpenConsole.exe" never change their inherited working directory? I guess they could change it to the system directory.

@vefatica
Copy link

vefatica commented Aug 2, 2021

I think there are situations in which windowsterminal.exe changes its CWD ... using "Open in Windows Terminal" with a re-used instance. IIRC, folks wanted new shells opened in "." to follow the "open in" mechanism around. (yes/no?)

@zadjii-msft
Copy link
Member

That's correct. When you've got the windowing behavior set to re-use an existing window (the second two options here:
image
), or when you do a wt -w N -- whatever.exe, we'll have the wt opening the commandline switch to the CWD of the source of the commandline, spawn that connection, then switch back. This is to make sure profiles that have their startingDirectory set to something like . work correctly.

@zadjii-msft zadjii-msft added Issue-Question For questions or discussion Product-Terminal The new Windows Terminal. Resolution-Answered Related to questions that have been answered labels Aug 2, 2021
@zomgrolf
Copy link
Author

zomgrolf commented Aug 2, 2021

So how do I force Windows Terminal to change the current working directory when I change the directory in the command prompt?

@vefatica
Copy link

vefatica commented Aug 2, 2021

So how do I force Windows Terminal to change the current working directory when I change the directory in the command prompt?

The short answer is you can't. Why would you want to?

@zomgrolf
Copy link
Author

zomgrolf commented Aug 2, 2021

Consider the following scenario:

  1. A developer launches the terminal from a directory that contains some build artifacts (e.g. CMake configuration cache or maybe some other files generated as a part of the build process, or perhaps not even build artifacts, it doesn't really matter). She or he does some work on the command line in that directory, then switches away to another directory on the filesystem.
  2. Over the course of the next hour(s), said developer launches a number of other terminal windows (possibly with tabs), working on some other (maybe related, maybe not) things in other areas of the filesystem.
  3. At some point (s)he decides to build the first project. Or maybe hands over the machine to another developer who needs to do a full rebuild. In this case, certain builds are done from scratch and involve wiping clean the entire build area and resyncing the source code and all dependencies. And so that other developer triggers a full rebuild -- except now the part that does the cleanup mysteriously fails, although it used to work fine. It's not immediately obvious what is the cause and why that directory could not be deleted -- it doesn't contain any executables/dlls that could be locked by a running process, none of the open command prompts have it set as their current directory.

That's because someone else started the terminal from that directory a few hours ago.

At this point your option is to look for the handle, then try to match the PID of the process that owns it with the right instance of the terminal and close it, otherwise you won't be able run the automated build process. Not the best user experience.

And if that first terminal was actually used to start some long running process (e.g. a regression suite for another build configuration, 8+ hours total and you're on hour 4), then you're out of luck.

Mind you, all of this works with the old conhost just fine -- there are no issues with deleting the starting directory, so from our point of view, this is effectively a regression.

Not sure how it fits with the plans for making Windows Terminal the default terminal in the future.

@zomgrolf
Copy link
Author

zomgrolf commented Aug 2, 2021

Now, it doesn't have to actually track the changes to the current directory in the shell. If there were an option to launch the terminal with the current working directory set to say, the system directory, but the current directory in the shell set to ".", then that would work just fine for me...

@vefatica
Copy link

vefatica commented Aug 2, 2021

Now, it doesn't have to actually track the changes to the current directory in the shell. If there were an option to launch the terminal with the current working directory set to say, the system directory, but the current directory in the shell set to ".", then that would work just fine for me...

I'm not sure what you're saying. If the terminal's CWD is the system directory, what is "."?

@DHowett
Copy link
Member

DHowett commented Aug 2, 2021

Terminal already has some provisions for caching that it thinks the current working directory should be, and it already forcibly resolves relative paths. It would be trivial for us to:

  1. Save the current WD as a path
  2. Change the WD to System32 or something
  3. Never call GetCurrentDirectoryW, and always rely on the saved value.

This would allow us to unlock the working directory and still work normally.

@zomgrolf
Copy link
Author

zomgrolf commented Aug 2, 2021

Now, it doesn't have to actually track the changes to the current directory in the shell. If there were an option to launch the terminal with the current working directory set to say, the system directory, but the current directory in the shell set to ".", then that would work just fine for me...

I'm not sure what you're saying. If the terminal's CWD is the system directory, what is "."?

The directory from which I want to launch the terminal. In the example at the top of the thread it is c:\temp\rmdir_test -- that's where I want to start my shell -- meanwhile, the current working directory for the Windows Terminal process (which apparently, does not matter to me in this scenario) should be set to the system directory.

@zomgrolf
Copy link
Author

zomgrolf commented Aug 2, 2021

Again, when I launch a new instance of the terminal from an arbitrary directory, I want either:

  1. For the CWD of the terminal process to track the current directory in the shell as I navigate around the file system, or:
  2. To set that CWD to something other than the starting directory (while still starting the shell in that directory), preferably the system directory, since I will never, ever want to delete that.

The reason is that, currently, if you launch the terminal from an arbitrary directory, you cannot delete it until you close that instance of the terminal, and for various reasons you might actually want to delete it at some point. This is even more confusing, if you started your terminal some time ago (or, for example, it was launched by someone else the previous week), because there is absolutely nothing in the UI that indicates/reminds you that this is the case.

@zadjii-msft
Copy link
Member

Okeydokey well it seems like Dustin's got a good idea up in #10849 (comment) so let's just do that.

@zadjii-msft zadjii-msft added Area-TerminalConnection Issues pertaining to the terminal<->backend connection interface Help Wanted We encourage anyone to jump in on these. Issue-Task It's a feature request, but it doesn't really need a major design. and removed Issue-Question For questions or discussion Resolution-Answered Related to questions that have been answered labels Aug 2, 2021
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Aug 2, 2021
@zadjii-msft zadjii-msft added this to the Terminal Backlog milestone Aug 2, 2021
@vefatica
Copy link

vefatica commented Aug 2, 2021

I think that's what @DHowett is talking about. It'd probably also make sense to ensure that OpenConsole.exe's CWD is something innocuous.

Off-topic, but somewhat interesting ... I never noticed until today that when a console is started, conhost.exe's CWD is c:\windows.

You can get some dicey situations in a console. Start a CMD.EXE console with CWD = directory_A. At the CMD prompt, issue CMD and then issue CD /D directory_B. Hand the machine over to someone else, and he won't be able to remove directory_A.

@eryksun
Copy link

eryksun commented Aug 2, 2021

Off-topic, but somewhat interesting ... I never noticed until today that when a console is started, conhost.exe's CWD is c:\windows.

When an application creates a console session at startup, or via AllocConsole(), the base API sends the request to the ConDrv device driver using an IOCTL, and the driver in turn spawns conhost.exe from kernel mode, with the working directory set to %SystemRoot%.

In Windows 10, conhost.exe can also be executed directly, which defaults to spawning cmd.exe as the client application, unless a different executable is passed in the command line string. I think for this case conhost.exe should query and save the working directory in order to spawn the client with the desired working directory, but then change its working directory to %SystemRoot%.

You can get some dicey situations in a console. Start a CMD.EXE console with CWD = directory_A. At the CMD prompt, issue CMD and then issue CD /D directory_B. Hand the machine over to someone else, and he won't be able to remove directory_A.

CMD uses the process working directory to track its CLI working directory, i.e. the directory that's set by chdir or cd. Interestingly, PowerShell set-location (i.e. cd or chdir) doesn't change the process working directory of the pwsh.exe process. It generalizes its working directory beyond filesystem directories (e.g. see get-psdrive), so it doesn't even bother to change the process working directory.

@zadjii-msft zadjii-msft removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Aug 3, 2021
@zadjii-msft
Copy link
Member

You know what, there's actually another issue on the repo that's effectively the same thing: #5506. Both threads have great discussion.

For housekeeping purposes though, I'm gonna close this one out as a dup of the other. Thanks all! /dup #5506

@ghost
Copy link

ghost commented Dec 9, 2021

Hi! We've identified this issue as a duplicate of another one that already exists on this Issue Tracker. This specific instance is being closed in favor of tracking the concern over on the referenced thread. Thanks for your report!

@ghost ghost closed this as completed Dec 9, 2021
@ghost ghost added the Resolution-Duplicate There's another issue on the tracker that's pretty much the same thing. label Dec 9, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-TerminalConnection Issues pertaining to the terminal<->backend connection interface Help Wanted We encourage anyone to jump in on these. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. Resolution-Duplicate There's another issue on the tracker that's pretty much the same thing.
Projects
None yet
Development

No branches or pull requests

5 participants