On 4/6/23 3:08 PM, Eric DeCosta wrote:
-----Original Message-----
From: Jeff Hostetler <git@xxxxxxxxxxxxxxxxx>
Sent: Monday, March 27, 2023 11:02 AM
To: Johannes Schindelin <Johannes.Schindelin@xxxxxx>; Eric DeCosta via
GitGitGadget <gitgitgadget@xxxxxxxxx>
Cc: git@xxxxxxxxxxxxxxx; Eric DeCosta <edecosta@xxxxxxxxxxxxx>
Subject: Re: [PATCH] fsmonitor: handle differences between Windows
named pipe functions
On 3/27/23 7:37 AM, Johannes Schindelin wrote:
Hi Eric,
On Fri, 24 Mar 2023, Eric DeCosta via GitGitGadget wrote:
From: Eric DeCosta <edecosta@xxxxxxxxxxxxx>
CreateNamedPipeW is perfectly happy accepting pipe names with
seemingly embedded escape charcters (e.g. \b), WaitNamedPipeW is not
and incorrectly returns ERROR_FILE_NOT_FOUND when clearly a named
pipe, succesfully created with CreateNamedPipeW, exists.
For example, this network path is problemmatic:
\\batfs-sb29-cifs\vmgr\sbs29\my_git_repo
In order to work around this issue, rather than using the path to the
worktree directly as the name of the pipe, instead use the hash of
the worktree path.
This is a rather large deviation from the other platforms, and it has
an unwanted side effect: Git for Windows' installer currently
enumerates the named pipes to figure out which FSMonitor instances
need to be stopped before upgrading. It has to do that because it
would otherwise be unable to overwrite the Git executable. And it
needs to know the paths [*1*] so that it can stop the FSMonitors
gracefully (as opposed to terminating them and risk interrupting them
while they serve a reply to a Git client).
A much less intrusive change (that would not break Git for Windows'
installer) would be to replace backslashes by forward slashes in the path.
Please do that instead.
Ciao,
Johannes
Footnote *1*: If you think that the Git for Windows installer could
simply enumerate the process IDs of the FSMonitor instances and then
look for their working directories: That is not a viable option. Not
only does the Windows-based FSMonitor specifically switch to the
parent directory (to avoid blocking the removal of a Git directory
merely by running the process in said directory), even worse: there is
no officially-sanctioned way to query a running process' current
working directory (the only way I know of involves injecting a remote
thread! Which will of course risk being labeled as malware by current anti-
malware solutions).
Agreed. Please use forward slashes.
Thanks,
Jeff
I have misdiagnosed the problem. Here are my most recent findings:
The problem is the leading double-slashes for repos that resolve to remote filesystems. i.e. if S:\myrepo resolves to \\some-server\some-dir\myrepo then the path passed to initialize_pipe_name is //some-server/some-dir/myrepo
Regardless of what type or how many slashes appear after \\.\pipe\ the pipe name, as reported from PowerShell, is always \\.\\pipe\\some-server\some-dir\myrepo and WaitNamedPipeW returns ERROR_FILE_NOT_FOUND
If I skip over the first leading slash an use /some-server/some-dir/myrepo I get the same pipe name as before, WaitNamedPipeW is happy and commands like git fsmonitor--daemon status correctly report that the daemon is watching the repo.
-Eric
The named pipe file system (NPFS) is a little "special". It is a flat
namespace and not hierarchical and not subject to the usual Win32
and/or NTFS limitations/quirks (such as restricted characters or legacy
filename suffixes). It is a single level dictionary, in a sense.
The local form is "\\.\pipe\<name>" and according to [1], the only
restriction is that <name> portion may not contain backslashes[1],
but I'm seeing lots of named pipes of the form "\\.\pipe\Winsock2\..."
on my Windows 10 system, so that restriction may have been lifted
since the documentation was last updated.
[1]
https://learn.microsoft.com/en-us/windows/win32/api/namedpipeapi/nf-namedpipeapi-createnamedpipew
Forward slashes (and now it seems backslashes) are not directory
separators -- they are just another character in the allowed char[256].
We tend to think of them as directory separators, but that is an
illusion. For example, in a CMD prompt:
dir \\.\pipe\\ /b
shows a simple list of all the named pipes on the system, including
some "Winsock2\CatalogChangeListener..." ones. However, any attempt
to list the contents of the "Winsock2" directory:
dir \\.\pipe\\Winsock2\\ /b
fails with a file not found error.
However, a simple wildcard lists them:
dir \\.\pipe\\Winsock2* /b
From PowerShell, we can see a complete list of pipes with:
(get-childitem \\.\pipe\).FullName
But we get a path does not exist with:
(get-childitem \\.\pipe\Winsock2\).FullName
However "get-childitem" is confused and reports "Winsock2" as
a directory multiple times, each with one item, when we do:
(get-childitem \\.\pipe)
(BTW There's also the "Pipelist" tool from SysInternals that shows
them as a simple list of names (some with the embedded backslashes).
In [1], it also says that CreateNamedPipeW() can only create a local
"\\.\pipe\<something>" pipe, so I wonder if CreateNamedPipeW() is
silently prefixing "\\.\pipe\" if necessary... I haven't had time
to try this.
Then WaitNamedPipeW() and/or CreateFileW() allows fully general
"\\<host>\<share>\<pathnames>", so the OS cannot do any implicit
fixup -- and these calls actually try to access the (intended)
network file.
I'm guessing here that this is the problem you've found.
If that is the case, we need to think about how to fix it
mainly because of what Johannes said about the installer
needing to properly shutdown running daemons during an upgrade.
Or rather, we will need to coordinate with the GFW installer.
Please let me know if any of this makes sense.
Thanks,
Jeff
