search

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
336 stars 65 forks source link

behavior of Accept() changed #21

Closed pmorjan closed 5 years ago

pmorjan commented 5 years ago

It seems that fda437ecbf04836c30bfb8c397ca7383c5d01a7e changed the behavior of the Accept method of vsock listeners. Accept is no longer a blocking operation. You can no longer simply loop on Accept() like in the example below. Accept always fails with EWOULDBLOCK. The demo in cmd/vcsp also fails. Is this intended? Thanks.

for {
    c, err := l.Accept()
    if err != nil {
        panic(err) // resource temporarily unavailable
    }
    go func() { doSomething(c) }()
}
mdlayher commented 5 years ago

Are you using Go 1.11.x? I haven't used this package myself in several months but I understand these changes were made to make integration with the runtime network poller possible.

/cc @aybabtme @mxpv

mxpv commented 5 years ago

Hey. Right, Accept is no longer blocking. Initially I did this changes because of https://github.com/mdlayher/vsock/issues/19

I think a correct approach of handling this would be check if returned error is temporary. Here is an example.

pmorjan commented 5 years ago

I understand the motivation for this PR. However I think it is worth mentioning that it's not only a bug fix as the title suggests. It now behaves differently to the net package. My fault not pointing that out earlier. Thanks.

mdlayher commented 5 years ago

I'm not closely following the work on integrating external types with the runtime network poller these days. I suspect that work would enable us to solve both use cases, because the runtime would allow the accept to block until a connection is received, and enable closing it later.

mdlayher commented 5 years ago

Fixed as of https://github.com/mdlayher/vsock/pull/26.