microsoft / go-winio

Win32 IO-related utilities for Go
MIT License
952 stars 182 forks source link

CloseWrite() is non-portable and may leave client connection hanging #257

Open nicks opened 2 years ago

nicks commented 2 years ago

We currently use winio's named pipe implementation to create http duplex sockets.

We use CloseWrite() to close the server side of the socket.

This works ok if the client is also using winio. But if the client uses a different named pipe implemenation, CloseWrite() does not behave correctly. (in particular, NodeJS/libuv's implementation)

My (very naive) understanding is the winio uses a 0-length message to signal the end of the socket. https://github.com/microsoft/go-winio/blob/d68e55cd0b80e6e3cac42e945f1cff6ddb257090/pipe.go#L161 which other libraries do not support.

Can you all advise on if there's something we should be doing differently here?

Thank you!


Detailed repro steps:

Here's how this breaks things in application-level code: https://github.com/apocas/dockerode/issues/534

Here's a more detailed repro case:

server side:

// main.go

package main

import (
    "log"
    "fmt"
    "github.com/Microsoft/go-winio"
)

type CloseWriter interface {
    CloseWrite() error
}

func main() {
    pipe, err := winio.ListenPipe("//./pipe/test-socket", &winio.PipeConfig{
        MessageMode:        true,
        InputBufferSize:    65536,
        OutputBufferSize:   65536,
    })
    if err != nil {
        log.Fatal(err)
    }
    fmt.Println("Accepting connections on //./pipe/test-socket")
    for {

        conn, err := pipe.Accept()
        if err != nil {
            log.Fatal(err)
      }
        fmt.Println("opened socket")
    go func() {

            conn.Write([]byte("HTTP/1.1 101 Web Socket Protocol Handshake\r\n" +
                "Upgrade: WebSocket\r\n" +
                "Connection: Upgrade\r\n" +
                "\r\n"));
            conn.Write([]byte("closing socket\r\n"))
            conn.(CloseWriter).CloseWrite()
            fmt.Println("closed socket")
        }()
    }
}

client side:

// index.js
const http = require('http');
const options = {
  headers: {
    'Connection': 'Upgrade',
    'Upgrade': 'tcp'
  },
  socketPath: '//./pipe/test-socket'
};

const req = http.request(options);
req.on('upgrade', (res, socket, upgradeHead) => {
    console.log('connected to socket');
    socket.allowHalfOpen = false;
    socket.on('data', (data) => {
        console.log('socket received data:', data.toString());
    });
    socket.on('end', () => {
        console.log('socket closed');
    });
});
req.end();

Then run (in separate terminals):

go run ./main.go
node index.js

expected behavior: WinIO should close the socket and nodejs should detect the 'end' event.

actual behavior: nodejs hangs indefinitely

Other notes: The workaround is to use Close() on the Go side instead of CloseWrite(). This closes the whole duplex (rather than closing just one side)

nicks commented 2 years ago

i also understand that this may end up being a libuv bug -- i couldn't find much documentation on the 0-length message approach to CloseWrite() that winio does, or if this is a generally accepted pattern.