microsoft / go-winio

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

HvsockConn shutdown bugfix, added .IsClosed() functions #231

Closed helsaawy closed 2 years ago

helsaawy commented 2 years ago

The PR fixes bugs with HvsockConn CloseRead/Write where shutdown is attempted on closed sockets, which returns a cryptic error: close hvsock [...]: shutdown: An operation was attempted on something that is not a socket, and where the (*HvsockConn).shutdown function was not respecting whether to close a socket for reading or writing, and only shutting down the pipe for reading.

Currently, (*Process).CloseStdin runs into two issues:

  1. It attempts to shutdown a closed socket, which then calls syscall.Shutdown on a null socket handle. This PR preemptively returns ErrFileClosed to prevent that, and allow upstream callers to check for the situation where the socket is already closed.
  2. (*HvsockConn).CloseWrite does not actually close the connection for writing, since (*HvsockConn).shutdown ignores its parameter and closes the socket only for reading. This prevents hcsshim from using CloseWrite (from within (*Process).CloseStdin) to stop the copy operation from upstream pipes into the processes std in.

This PR fixes those issues.

Additionally, two functions have been added:

  1. (*HvsockConn).CloseReadWrite to expose shutting down both ends of the socket.
  2. (*win32File).IsClosed (and therefore win32Pipe and win32MessageBytePipe) and (*HvsockConn).IsClosed and to check if the socket/pipe has already been closed, since the structs already track internally if they have been closed or not.

Signed-off-by: Hamza El-Saawy hamzaelsaawy@microsoft.com

ambarve commented 2 years ago

Is this change fixing some bug? Can you also add some more context on that please?

helsaawy commented 2 years ago

@ambarve I added more context, ptal