Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vsock: enable timeouts also on listener connections #16

Merged
merged 2 commits into from
Aug 6, 2018
Merged

vsock: enable timeouts also on listener connections #16

merged 2 commits into from
Aug 6, 2018

Conversation

pmorjan
Copy link
Contributor

@pmorjan pmorjan commented Aug 5, 2018

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
Copy link
Contributor Author

pmorjan commented Aug 5, 2018

ok, it's not that simple.

@pmorjan pmorjan closed this Aug 5, 2018
@mdlayher
Copy link
Owner

mdlayher commented Aug 6, 2018

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
Copy link
Contributor Author

pmorjan commented Aug 6, 2018

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
Copy link
Owner

mdlayher commented Aug 6, 2018

Oh! I apologize, I misread your original patch. We support timeouts now on Go 1.11+ for connections that are dialed out: f68ad55.

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
Copy link
Owner

mdlayher commented Aug 6, 2018

Did you close this because of the CI failure?

@mdlayher mdlayher reopened this Aug 6, 2018
@mdlayher
Copy link
Owner

mdlayher commented Aug 6, 2018

Reopening, if you can fix the tests by making a similar patch to #14, I'd be happy to merge.

@mdlayher
Copy link
Owner

mdlayher commented Aug 6, 2018

LGTM thanks.

@mdlayher mdlayher merged commit e73e6ad into mdlayher:master Aug 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants