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.)
Starting in go1.9, Golang lowers the timer frequency when not required
reducing CPU usage. This workaround prevents Golang from doing so,
and since the initial issue was fixed, I see no perf regressions.
This does not need to be conditional based on Golang verison, as the issue
being worked around was only in go1.6.
Signed-off-by: Darren Stahl <darst@microsoft.com>
This change resolves the issue where Accept() returns ERROR_NO_DATA
because the client closes the connection immediately. It resolves
the race by ignoring that particular connection and waiting on a
new one.
Kevin Parsons