(I believe this change also should be used instead of PR #75 -- that PR is potentially extraordinarily buggy.)

This changes a few things to try to ensure that we never wind up with a result of ERROR_FILE_NOT_FOUND, due to a race between closing the last pipe instance and opening the next.

First we keep an "open" client instance (unused) while the listener is open, so that we are guaranteed to always have an active pipe instance. This means attempts to open while no other instances exist result in ERROR_PIPE_BUSY instead of ERROR_FILE_NOT_FOUND.

Second we have changed the loop for dialing to eliminate a race condition that is more or less inherent in WaitNamedPipe when synchronizing with CreateFile. The real timeout needs to be some larger value than the WaitNamedPipe timeout, and furthermore WaitNamedPipe is not very nice with the Go runtime, since it is a blocking system call. Instead we just put the goroutine to sleep for 10 milliseconds, and keep retrying the CreateFile until the maximum timeout is reached. If no timeout is specified we assume a reasonable and large default of 5 seconds, which is similar to a TCP connection timeout.

This isn't perfect, as a client attempting to connect to an extremely busy pipe server can be starved out by other clients coming in while it is in that brief sleep, but this potential race was already present with WaitNamedPipe. The numerous retries (by default 500 retries!) mean its pretty unlikely to occur, and if a single client hits the race once, it has an excellent chance of getting in the next cycle.

(A real "fix" that is completely race free and fair would require changes in the underlying Named Pipe implementation, or some other kind of external coordination.)

If the named pipe server does not have an outstanding call to ConnectNamedPipe() at the time the client calls CreateFile() or WaitNamedPipe(), the client can receive ERROR_FILE_NOT_FOUND and ERROR_BAD_PATHNAME errors. When that's the case, there is no choice but to retry after a short delay in the hope that next time, the server will be able to accept the connection.

This error occurs infrequently in low traffic situations, but it manifests quite reliably when clients open concurrent connections and/or open many short-lived connections in a short period of time.

Fixes #67.

In order to upgrade to go1.8 all IO operations including previous pending IO must have the deadline set when SetReadDeadline or SetWriteDeadline is called.

This moves the timeout cancelation responsibility out of the wait routine and into timers in the deadline code.

@jstarks PTAL

Signed-off-by: Darren Stahl [email protected]

Hello!

It looks like there's a race condition created in (*win32File).closeHandle() where multiple concurrent goroutines are accessing the (*win32File).closing field. This shows up in the go race detector. This change adds a lock around the closing field to remove the race.

Thanks! Sam

It is convenient to be able to pull off the underlying file handle to pass to other Win32 functions that aren't supported by this library.

This change adds an Fd() uintptr method to the file implementation (matches the os.File method of the same name).

Currently there is some work in progress (https://github.com/portainer/portainer/pull/1186) to add mapped named pipe support into Portainer so it will be easier to run it in a Windows container ( docker run -u ContainerAdministrator -it -v //./pipe/docker_engine://./pipe/docker_engine -p 9000:9000 stefanscherer/portainer:insider -H npipe:////./pipe/docker_engine )

We're using winio.DialPipe() to connect to the Docker engine from inside a microsoft/nanoserver-insider container.

At the moment we see problems in Portainer as it sometimes makes multiple requests requested by the web frontend. Here's a WireShark screenshot with the problem.

The two /api/endpoints/1/docker/xx GET requests use separate connections and run in parallel. One of the calls aborts with "open //./pipe/docker_engine: The system cannot find the file specified."

I have seen the check for cERROR_PIPE_BUSY and first thought that createFile() returns cERROR_FILE_NOT_FOUND and aborts immediatelly.

We are using the default timeout right now

    return winio.DialPipe(namedPipePath, nil)

but I also tried giving a 5 second timeout

    timeout := 5000 * time.Millisecond
    return winio.DialPipe(namedPipePath, &timeout)

Digging deeper it seems like the waitNamedPipe() just does not wait long enough or cannot start wait for the pipe.

Adding some Println in DialPipe I now see this running Portainer in the second DialPipe call:

DialPipe createFile err All pipe instances are busy. DialPipe waitNamedPipe ms 4998 DialPipe waitNamedPipe returns err The system cannot find the file specified. DialPipe returns err The system cannot find the file specified.

The problem is that waitNamedPipe returns immediatelly and does not wait the given 5000 milliseconds.

How can we make DialPipe more reliable?

Using Hyper-V sockets requires registering the application in the Windows Registry.

This PR adds (de)registration functionality for an application name/GUID.

This PR is split from #235

Signed-off-by: Hamza El-Saawy [email protected]

When opening a disk for detach mode no parameters are required. This regression was introduced when the OpenVirtualDisk method was added. Moving the code back to the previous openVirtualDisk internal method so we can pass nil parameters.

Signed-off-by: Justin Terry (VM) [email protected]

The race usually happens when closeHandle() and prepareIo() are called concurrently; the former tries to set closing to true the latter tries to read its value.

In order to avoid this issue, we added a lock around the variable.

Signed-off-by: Kenfe-Mickael Laventure [email protected]

fixes #41

As far as I can tell, this should contain all the runtime.KeepAlive calls needed to conform to the new argument liveness guarantees in Go1.8.x

It turns out that the main issue was due to passing a Go pointer via native calls to the ioCompletionProcessor, causing the new garbage collection rules to cleanup the struct containing the channel even though it did not have a finalizer.

/cc @jhowardmsft @jstarks PTAL

Thanks @mappu for the initial investigation and repro test.

This also updates zsyscall_windows.go with the new 1.8 go generated code, though it is not related to the KeepAlive, it should be done with the update to 1.8.

Signed-off-by: Darren Stahl [email protected]

This get-only API will be useful for detecting when a file has multiple links to it.

Specifically, I plan to use this to provide an optimisation for https://github.com/microsoft/hcsshim/pull/901 (or afterwards, it's not a blocker...) by only attempting to recognise hard-links for files which have multiple links.

See https://github.com/TBBle/hcsshim/compare/base-layer-manipulation...TBBle:hack-base-layer-manipulation-with-branched-go-winio for the example use-case.

Is it possible to set FILE_FLAG_FIRST_PIPE_INSTANCE when creating listener. It seems theres no API exposing this when calling winio.ListenPipe(...). We need this functionality as it will prevent Instance Creation Race Conditions as detailed here https://csandker.io/2021/01/10/Offensive-Windows-IPC-1-NamedPipes.html#instance-creation-race-condition.

I noticed the comment on listener call here https://github.com/microsoft/go-winio/blob/650a2e464c7458a1d355d7bb5affad1eec1bfbb6/pipe.go#L444

Is this enforced? From testing I noticed if I try to run a second listener with same pipe, same configs it gave me open \\.\pipe\pipename: Access is denied.

Does this ensure no instances of the pipe were created before me?

• follow-up to https://github.com/microsoft/go-winio/pull/266

go.mod: golang.org/x/sys v0.1.0

The golang.org/x/,, projects now tag releases, so updating to the first tagged release.

go.mod: github.com/sirupsen/logrus v1.9.0

Updating to the current release, which has been out for a while, and most projects have already updated to this version, so let's update it here as well to reduces the number of versions in the dependency tree.

full diff: https://github.com/sirupsen/logrus/compare/v1.7.0...v1.9.0

It looks like for the purpose of the example "a" logger was needed, so not strictly logrus (probably fmt.Println() would've worked even).

This patch removes logrus as dependency for the example. The only remaining use of logrus in this repository is now in the pkg/etc/etwlogrus package, which does need logrus, but (possibly) could become its own module if we want to remove logrus as dependency of go-winio itself.

This removes golang.org/x/tools/cmd/stringer as a direct dependency of this module, instead using "go install" to install the required tool as part of go generate. I opted for pinning to a fixed version for reproducibility, but we could switch to @latest if there's no concerns for getting newer versions.

With this patch:

which stringer
# (no output)

go generate ./pkg/guid
which stringer
/Users/thajeztah/go/bin/stringer

New RemoveAll function intended to be a replacement for os.RemoveAll usage and be tailored to Windows scenarios.

Most importantly, this function works with []uint16 instead of string. This allows it to properly delete Windows filesystem trees that contain files with invalid UTF-16 paths.

Signed-off-by: Kevin Parsons [email protected]