Improve error return from AnonDialer on Windows

kevpar · kevpar · commit daf12cd19449 · 2019-07-30T17:20:37.000-07:00
AnonDialer will now return a "not found" error if the pipe is not found
before the timeout is reached. If the pipe exists but the timeout is
reached while attempting to connect, the timeout error will still be
returned.

This will allow the error handling logic to work properly when
connecting to the shim log pipe. An error message is only logged if the
error is not "not found", so now log noise from log pipes that were
never intended to be created by the shim will be hidden.

This change also cleans up the control flow for AnonDialer on Windows.
The new code should be more easily readable, but the only semantic
change is the error return value change.

Signed-off-by: Kevin Parsons &lt;kevpar@microsoft.com&gt;
diff --git a/runtime/v2/shim/util_windows.go b/runtime/v2/shim/util_windows.go
@@ -57,35 +57,33 @@ func SocketAddress(ctx context.Context, id string) (string, error) {
 
 // AnonDialer returns a dialer for a npipe
 func AnonDialer(address string, timeout time.Duration) (net.Conn, error) {
-	var c net.Conn
-	var lastError error
-	timedOutError := errors.Errorf("timed out waiting for npipe %s", address)
-	start := time.Now()
+	ctx, cancel := context.WithTimeout(context.Background(), timeout)
+	defer cancel()
+
+	// If there is nobody serving the pipe we limit the timeout for this case to
+	// 5 seconds because any shim that would serve this endpoint should serve it
+	// within 5 seconds.
+	serveTimer := time.NewTimer(5 * time.Second)
+	defer serveTimer.Stop()
 	for {
-		remaining := timeout - time.Since(start)
-		if remaining <= 0 {
-			lastError = timedOutError
-			break
-		}
-		c, lastError = winio.DialPipe(address, &remaining)
-		if lastError == nil {
-			break
-		}
-		if !os.IsNotExist(lastError) {
-			break
-		}
-		// There is nobody serving the pipe. We limit the timeout for this case
-		// to 5 seconds because any shim that would serve this endpoint should
-		// serve it within 5 seconds. We use the passed in timeout for the
-		// `DialPipe` timeout if the pipe exists however to give the pipe time
-		// to `Accept` the connection.
-		if time.Since(start) >= 5*time.Second {
-			lastError = timedOutError
-			break
+		c, err := winio.DialPipeContext(ctx, address)
+		if err != nil {
+			if os.IsNotExist(err) {
+				select {
+				case <-serveTimer.C:
+					return nil, errors.Wrap(os.ErrNotExist, "pipe not found before timeout")
+				default:
+					// Wait 10ms for the shim to serve and try again.
+					time.Sleep(10 * time.Millisecond)
+					continue
+				}
+			} else if err == context.DeadlineExceeded {
+				return nil, errors.Wrapf(err, "timed out waiting for npipe %s", address)
+			}
+			return nil, err
 		}
-		time.Sleep(10 * time.Millisecond)
+		return c, nil
 	}
-	return c, lastError
 }
 
 // NewSocket returns a new npipe listener
