mdlayher / vsock

Package vsock provides access to Linux VM sockets (AF_VSOCK) for communication between a hypervisor and its virtual machines. MIT Licensed.
MIT License
328 stars 65 forks source link

vsock: enable timeouts also on listener connections #16

Closed pmorjan closed 6 years ago

pmorjan commented 6 years ago

conn.SetDeadline() seems to work only on client connections. Trying to set a deadline on a listener connection always fails with "file type does not support deadline" It seems that setting the FD created by Accept4() into non blocking mode solves the issue. Thanks.

pmorjan commented 6 years ago

ok, it's not that simple.

mdlayher commented 6 years ago

Out of curiosity, what's the use case for this? https://golang.org/pkg/net/#Listener doesn't seem to expose any method of setting timeouts.

pmorjan commented 6 years ago

My (probably invalid) use case is similar what ReadTimeout in https://golang.org/pkg/net/http/#Server does. After accepting a client connection the listener expects some data sent from the client within a given time frame. If that doesn't arrive the connection is closed to free resources.

for {
   c, err := l.Accept()
   if err != nil {
       log.Printf("Accept failed: %v", err)
       return
   }   

   go func() {
       buf := make([]byte, 4096)
       if err := c.SetReadDeadline(time.Now().Add(time.Millisecond*100)); err != nil {
           log.Print(err)
       }   
       n, err := c.Read(buf)
       if err != nil {
           log.Printf("Read failed: %v", err)
           c.Close()
           return
       }   
      ...

With my little patch to vsock the error message on Read() for clients not sending any data is Read failed: read vsock:vm(3200420455):1: i/o timeout. Thanks.

mdlayher commented 6 years ago

Oh! I apologize, I misread your original patch. We support timeouts now on Go 1.11+ for connections that are dialed out: https://github.com/mdlayher/vsock/commit/f68ad5598dc650e9b3697dbd1ad86c8380c1624b.

But it didn't occur to me that the same change should be applied to newly accepted listener connections.

At a glance your patch does seem correct. Did you run into a problem with it?

mdlayher commented 6 years ago

Did you close this because of the CI failure?

mdlayher commented 6 years ago

Reopening, if you can fix the tests by making a similar patch to https://github.com/mdlayher/vsock/pull/14, I'd be happy to merge.

mdlayher commented 6 years ago

LGTM thanks.