subject:"\[go\-nuts\] Potential race condition in prototypical network server code"

Re: [go-nuts] Potential race condition in prototypical network server code

2017-08-24 Thread Tom Payne

Ah, cool. So graceful shutdown can be achieved by calling Close() on the 
net.Listener. Nice!

Thank you :)

Tom


On Thursday, August 24, 2017 at 8:20:34 PM UTC+2, Jan Mercl wrote:
>
> net.Listener has a Close method that will make Accept return an error. 
> net.Listener.Accept is not equal to accept(2).
>
> On Thu, Aug 24, 2017, 20:13 Tom Payne  
> wrote:
>
>> Thanks. Looking at the man page for Linux's accept(2) 
>> http://man7.org/linux/man-pages/man2/accept.2.html it seems that 
>> l.Accept is unlikely to ever return an error.
>>
>> accept(2) returns an error if either the connection is incorrectly set up 
>> somehow, or if a system call is interrupted by a signal, or if the process 
>> or system runs out of resources (either file handles or memory).
>>
>> Setup problems should occur immediately (before any client connection is 
>> actually accepted) so there is no race there. It looks to me that EINTR is 
>> *not* handled by Go's runtime so real world (not prototypical) server code 
>> needs to handle EINTR, but there is no race condition in the code (in the 
>> case of a signal, the function will terminate without leaking resources). 
>> In the case of resource exhaustion it's hard to implement sane behaviour in 
>> any case.
>>
>> Thanks again for the discussion - I'm learning a lot here. Is there 
>> anything above that is not correct?
>>
>> Cheers,
>> Tom
>>
>>
>> On Thursday, August 24, 2017 at 7:56:22 PM UTC+2, Jan Mercl wrote:
>>
>>> The wg.Wait will be executed after l.Accept returns an error. It's 
>>> purpose is to wait for the completions of all handlers invoked in the go 
>>> statement that did not finished already.
>>>
>>> On Thu, Aug 24, 2017, 19:49 Tom Payne  wrote:
>>>
 Awesome, thanks Jan for the fast and clear response.

 In fact, the for {} is an infinite loop so wg.Wait() will never be 
 reached and serve() will never terminate. Correct?

 I guess I over-read how much this prototypical code was representative 
 of a real server loop. Sorry Dave!

 Tom

 On Thursday, August 24, 2017 at 7:41:59 PM UTC+2, Jan Mercl wrote:

> No, wg.Add cannot "switch" to wg.Wait, they're both in the samr 
> goroutine, the go statement will be always the next one to execute after 
> wg.Add within serve().
>
> On Thu, Aug 24, 2017, 19:29 Tom Payne  wrote:
>
 I'm not singling out Dave Cheney here, I'd just like to check my 
>> understanding of Go's resource handling and concurrency.
>>
>> In this blog post a "prototypical network server" is presented:
>>
>>https://dave.cheney.net/2017/08/20/context-isnt-for-cancellation
>>
>> Code:
>>
>> func serve(l net.Listener) error {
>> var wg sync.WaitGroup
>> var conn net.Conn
>> var err error
>> for {
>> conn, err = l.Accept()
>> if err != nil {
>> break
>> }
>> wg.Add(1)
>> go func(c net.Conn) {
>> defer wg.Done()
>> handle(c)
>> }(conn)
>> }
>> wg.Wait()
>> return err
>> }
>>
>> My understanding is that this contains a race condition. 
>> Specifically, a goroutine switch can occur at the entry to wg.Add(1), 
>> switching to wg.Wait() in the main goroutine. At this point, a 
>> connection 
>> has been accepted, but the WaitGroup counter has not been incremented, 
>> so 
>> the serve function will terminate while silently dropping a network 
>> connection (the connection will be accepted, but never handled, so will 
>> probably eventually time out on the client side and leak a file 
>> descriptor 
>> on the server side).
>>
>> Is this understanding correct? Furthermore, I suspect that it's 
>> impossible to implement race-free graceful termination using only 
>> sync.WaitGroups. Is it actually possible to do so?
>>
>> Thanks for any correction, and apologies to Dave for picking on his 
>> code.
>>
>> Cheers,
>> Tom
>>
>> -- 
>> You received this message because you are subscribed to the Google 
>> Groups "golang-nuts" group.
>>
> To unsubscribe from this group and stop receiving emails from it, send 
>> an email to golang-nuts...@googlegroups.com.
>
>
>> For more options, visit https://groups.google.com/d/optout.
>>
> -- 
>
> -j
>
 -- 
 You received this message because you are subscribed to the Google 
 Groups "golang-nuts" group.
 To unsubscribe from this group and stop receiving emails from it, send 
 an email to golang-nuts...@googlegroups.com.
 For more options, visit https://groups.google.com/d/optout.

>>> -- 
>>>
>>> -j
>>>
>>

Re: [go-nuts] Potential race condition in prototypical network server code

2017-08-24 Thread Jan Mercl

net.Listener has a Close method that will make Accept return an error.
net.Listener.Accept is not equal to accept(2).

On Thu, Aug 24, 2017, 20:13 Tom Payne  wrote:

> Thanks. Looking at the man page for Linux's accept(2)
> http://man7.org/linux/man-pages/man2/accept.2.html it seems that l.Accept
> is unlikely to ever return an error.
>
> accept(2) returns an error if either the connection is incorrectly set up
> somehow, or if a system call is interrupted by a signal, or if the process
> or system runs out of resources (either file handles or memory).
>
> Setup problems should occur immediately (before any client connection is
> actually accepted) so there is no race there. It looks to me that EINTR is
> *not* handled by Go's runtime so real world (not prototypical) server code
> needs to handle EINTR, but there is no race condition in the code (in the
> case of a signal, the function will terminate without leaking resources).
> In the case of resource exhaustion it's hard to implement sane behaviour in
> any case.
>
> Thanks again for the discussion - I'm learning a lot here. Is there
> anything above that is not correct?
>
> Cheers,
> Tom
>
>
> On Thursday, August 24, 2017 at 7:56:22 PM UTC+2, Jan Mercl wrote:
>
>> The wg.Wait will be executed after l.Accept returns an error. It's
>> purpose is to wait for the completions of all handlers invoked in the go
>> statement that did not finished already.
>>
>> On Thu, Aug 24, 2017, 19:49 Tom Payne  wrote:
>>
>>> Awesome, thanks Jan for the fast and clear response.
>>>
>>> In fact, the for {} is an infinite loop so wg.Wait() will never be
>>> reached and serve() will never terminate. Correct?
>>>
>>> I guess I over-read how much this prototypical code was representative
>>> of a real server loop. Sorry Dave!
>>>
>>> Tom
>>>
>>> On Thursday, August 24, 2017 at 7:41:59 PM UTC+2, Jan Mercl wrote:
>>>
 No, wg.Add cannot "switch" to wg.Wait, they're both in the samr
 goroutine, the go statement will be always the next one to execute after
 wg.Add within serve().

 On Thu, Aug 24, 2017, 19:29 Tom Payne  wrote:

>>> I'm not singling out Dave Cheney here, I'd just like to check my
> understanding of Go's resource handling and concurrency.
>
> In this blog post a "prototypical network server" is presented:
>
>https://dave.cheney.net/2017/08/20/context-isnt-for-cancellation
>
> Code:
>
> func serve(l net.Listener) error {
> var wg sync.WaitGroup
> var conn net.Conn
> var err error
> for {
> conn, err = l.Accept()
> if err != nil {
> break
> }
> wg.Add(1)
> go func(c net.Conn) {
> defer wg.Done()
> handle(c)
> }(conn)
> }
> wg.Wait()
> return err
> }
>
> My understanding is that this contains a race condition. Specifically,
> a goroutine switch can occur at the entry to wg.Add(1), switching to
> wg.Wait() in the main goroutine. At this point, a connection has been
> accepted, but the WaitGroup counter has not been incremented, so the serve
> function will terminate while silently dropping a network connection (the
> connection will be accepted, but never handled, so will probably 
> eventually
> time out on the client side and leak a file descriptor on the server 
> side).
>
> Is this understanding correct? Furthermore, I suspect that it's
> impossible to implement race-free graceful termination using only
> sync.WaitGroups. Is it actually possible to do so?
>
> Thanks for any correction, and apologies to Dave for picking on his
> code.
>
> Cheers,
> Tom
>
> --
> You received this message because you are subscribed to the Google
> Groups "golang-nuts" group.
>
 To unsubscribe from this group and stop receiving emails from it, send
> an email to golang-nuts...@googlegroups.com.


> For more options, visit https://groups.google.com/d/optout.
>
 --

 -j

>>> --
>>> You received this message because you are subscribed to the Google
>>> Groups "golang-nuts" group.
>>> To unsubscribe from this group and stop receiving emails from it, send
>>> an email to golang-nuts...@googlegroups.com.
>>> For more options, visit https://groups.google.com/d/optout.
>>>
>> --
>>
>> -j
>>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-nuts" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-nuts+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>
-- 

-j

-- 
You received this message because you are subscribed to the

Re: [go-nuts] Potential race condition in prototypical network server code

2017-08-24 Thread Tom Payne

Thanks. Looking at the man page for Linux's 
accept(2) http://man7.org/linux/man-pages/man2/accept.2.html it seems that 
l.Accept is unlikely to ever return an error.

accept(2) returns an error if either the connection is incorrectly set up 
somehow, or if a system call is interrupted by a signal, or if the process 
or system runs out of resources (either file handles or memory).

Setup problems should occur immediately (before any client connection is 
actually accepted) so there is no race there. It looks to me that EINTR is 
*not* handled by Go's runtime so real world (not prototypical) server code 
needs to handle EINTR, but there is no race condition in the code (in the 
case of a signal, the function will terminate without leaking resources). 
In the case of resource exhaustion it's hard to implement sane behaviour in 
any case.

Thanks again for the discussion - I'm learning a lot here. Is there 
anything above that is not correct?

Cheers,
Tom

On Thursday, August 24, 2017 at 7:56:22 PM UTC+2, Jan Mercl wrote:
>
> The wg.Wait will be executed after l.Accept returns an error. It's purpose 
> is to wait for the completions of all handlers invoked in the go statement 
> that did not finished already.
>
> On Thu, Aug 24, 2017, 19:49 Tom Payne  
> wrote:
>
>> Awesome, thanks Jan for the fast and clear response.
>>
>> In fact, the for {} is an infinite loop so wg.Wait() will never be 
>> reached and serve() will never terminate. Correct?
>>
>> I guess I over-read how much this prototypical code was representative of 
>> a real server loop. Sorry Dave!
>>
>> Tom
>>
>> On Thursday, August 24, 2017 at 7:41:59 PM UTC+2, Jan Mercl wrote:
>>
>>> No, wg.Add cannot "switch" to wg.Wait, they're both in the samr 
>>> goroutine, the go statement will be always the next one to execute after 
>>> wg.Add within serve().
>>>
>>> On Thu, Aug 24, 2017, 19:29 Tom Payne  wrote:
>>>
>> I'm not singling out Dave Cheney here, I'd just like to check my 
 understanding of Go's resource handling and concurrency.

 In this blog post a "prototypical network server" is presented:

https://dave.cheney.net/2017/08/20/context-isnt-for-cancellation

 Code:

 func serve(l net.Listener) error {
 var wg sync.WaitGroup
 var conn net.Conn
 var err error
 for {
 conn, err = l.Accept()
 if err != nil {
 break
 }
 wg.Add(1)
 go func(c net.Conn) {
 defer wg.Done()
 handle(c)
 }(conn)
 }
 wg.Wait()
 return err
 }

 My understanding is that this contains a race condition. Specifically, 
 a goroutine switch can occur at the entry to wg.Add(1), switching to 
 wg.Wait() in the main goroutine. At this point, a connection has been 
 accepted, but the WaitGroup counter has not been incremented, so the serve 
 function will terminate while silently dropping a network connection (the 
 connection will be accepted, but never handled, so will probably 
 eventually 
 time out on the client side and leak a file descriptor on the server side).

 Is this understanding correct? Furthermore, I suspect that it's 
 impossible to implement race-free graceful termination using only 
 sync.WaitGroups. Is it actually possible to do so?

 Thanks for any correction, and apologies to Dave for picking on his 
 code.

 Cheers,
 Tom

 -- 
 You received this message because you are subscribed to the Google 
 Groups "golang-nuts" group.

>>> To unsubscribe from this group and stop receiving emails from it, send 
 an email to golang-nuts...@googlegroups.com.
>>>
>>>
 For more options, visit https://groups.google.com/d/optout.

>>> -- 
>>>
>>> -j
>>>
>> -- 
>> You received this message because you are subscribed to the Google Groups 
>> "golang-nuts" group.
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to golang-nuts...@googlegroups.com .
>> For more options, visit https://groups.google.com/d/optout.
>>
> -- 
>
> -j
>

-- 
You received this message because you are subscribed to the Google Groups 
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to golang-nuts+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [go-nuts] Potential race condition in prototypical network server code

2017-08-24 Thread Jan Mercl

The wg.Wait will be executed after l.Accept returns an error. It's purpose
is to wait for the completions of all handlers invoked in the go statement
that did not finished already.

On Thu, Aug 24, 2017, 19:49 Tom Payne  wrote:

> Awesome, thanks Jan for the fast and clear response.
>
> In fact, the for {} is an infinite loop so wg.Wait() will never be reached
> and serve() will never terminate. Correct?
>
> I guess I over-read how much this prototypical code was representative of
> a real server loop. Sorry Dave!
>
> Tom
>
> On Thursday, August 24, 2017 at 7:41:59 PM UTC+2, Jan Mercl wrote:
>
>> No, wg.Add cannot "switch" to wg.Wait, they're both in the samr
>> goroutine, the go statement will be always the next one to execute after
>> wg.Add within serve().
>>
>> On Thu, Aug 24, 2017, 19:29 Tom Payne  wrote:
>>
> I'm not singling out Dave Cheney here, I'd just like to check my
>>> understanding of Go's resource handling and concurrency.
>>>
>>> In this blog post a "prototypical network server" is presented:
>>>
>>>https://dave.cheney.net/2017/08/20/context-isnt-for-cancellation
>>>
>>> Code:
>>>
>>> func serve(l net.Listener) error {
>>> var wg sync.WaitGroup
>>> var conn net.Conn
>>> var err error
>>> for {
>>> conn, err = l.Accept()
>>> if err != nil {
>>> break
>>> }
>>> wg.Add(1)
>>> go func(c net.Conn) {
>>> defer wg.Done()
>>> handle(c)
>>> }(conn)
>>> }
>>> wg.Wait()
>>> return err
>>> }
>>>
>>> My understanding is that this contains a race condition. Specifically, a
>>> goroutine switch can occur at the entry to wg.Add(1), switching to
>>> wg.Wait() in the main goroutine. At this point, a connection has been
>>> accepted, but the WaitGroup counter has not been incremented, so the serve
>>> function will terminate while silently dropping a network connection (the
>>> connection will be accepted, but never handled, so will probably eventually
>>> time out on the client side and leak a file descriptor on the server side).
>>>
>>> Is this understanding correct? Furthermore, I suspect that it's
>>> impossible to implement race-free graceful termination using only
>>> sync.WaitGroups. Is it actually possible to do so?
>>>
>>> Thanks for any correction, and apologies to Dave for picking on his code.
>>>
>>> Cheers,
>>> Tom
>>>
>>> --
>>> You received this message because you are subscribed to the Google
>>> Groups "golang-nuts" group.
>>>
>> To unsubscribe from this group and stop receiving emails from it, send an
>>> email to golang-nuts...@googlegroups.com.
>>
>>
>>> For more options, visit https://groups.google.com/d/optout.
>>>
>> --
>>
>> -j
>>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-nuts" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-nuts+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>
-- 

-j

-- 
You received this message because you are subscribed to the Google Groups 
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to golang-nuts+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [go-nuts] Potential race condition in prototypical network server code

2017-08-24 Thread Tom Payne

Awesome, thanks Jan for the fast and clear response.

In fact, the for {} is an infinite loop so wg.Wait() will never be reached 
and serve() will never terminate. Correct?

I guess I over-read how much this prototypical code was representative of a 
real server loop. Sorry Dave!

Tom

On Thursday, August 24, 2017 at 7:41:59 PM UTC+2, Jan Mercl wrote:
>
> No, wg.Add cannot "switch" to wg.Wait, they're both in the samr goroutine, 
> the go statement will be always the next one to execute after wg.Add within 
> serve().
>
> On Thu, Aug 24, 2017, 19:29 Tom Payne  
> wrote:
>
>> I'm not singling out Dave Cheney here, I'd just like to check my 
>> understanding of Go's resource handling and concurrency.
>>
>> In this blog post a "prototypical network server" is presented:
>>
>>https://dave.cheney.net/2017/08/20/context-isnt-for-cancellation
>>
>> Code:
>>
>> func serve(l net.Listener) error {
>> var wg sync.WaitGroup
>> var conn net.Conn
>> var err error
>> for {
>> conn, err = l.Accept()
>> if err != nil {
>> break
>> }
>> wg.Add(1)
>> go func(c net.Conn) {
>> defer wg.Done()
>> handle(c)
>> }(conn)
>> }
>> wg.Wait()
>> return err
>> }
>>
>> My understanding is that this contains a race condition. Specifically, a 
>> goroutine switch can occur at the entry to wg.Add(1), switching to 
>> wg.Wait() in the main goroutine. At this point, a connection has been 
>> accepted, but the WaitGroup counter has not been incremented, so the serve 
>> function will terminate while silently dropping a network connection (the 
>> connection will be accepted, but never handled, so will probably eventually 
>> time out on the client side and leak a file descriptor on the server side).
>>
>> Is this understanding correct? Furthermore, I suspect that it's 
>> impossible to implement race-free graceful termination using only 
>> sync.WaitGroups. Is it actually possible to do so?
>>
>> Thanks for any correction, and apologies to Dave for picking on his code.
>>
>> Cheers,
>> Tom
>>
>> -- 
>> You received this message because you are subscribed to the Google Groups 
>> "golang-nuts" group.
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to golang-nuts...@googlegroups.com .
>> For more options, visit https://groups.google.com/d/optout.
>>
> -- 
>
> -j
>

-- 
You received this message because you are subscribed to the Google Groups 
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to golang-nuts+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [go-nuts] Potential race condition in prototypical network server code

2017-08-24 Thread Jan Mercl

No, wg.Add cannot "switch" to wg.Wait, they're both in the samr goroutine,
the go statement will be always the next one to execute after wg.Add within
serve().

On Thu, Aug 24, 2017, 19:29 Tom Payne  wrote:

> I'm not singling out Dave Cheney here, I'd just like to check my
> understanding of Go's resource handling and concurrency.
>
> In this blog post a "prototypical network server" is presented:
>
>https://dave.cheney.net/2017/08/20/context-isnt-for-cancellation
>
> Code:
>
> func serve(l net.Listener) error {
> var wg sync.WaitGroup
> var conn net.Conn
> var err error
> for {
> conn, err = l.Accept()
> if err != nil {
> break
> }
> wg.Add(1)
> go func(c net.Conn) {
> defer wg.Done()
> handle(c)
> }(conn)
> }
> wg.Wait()
> return err
> }
>
> My understanding is that this contains a race condition. Specifically, a
> goroutine switch can occur at the entry to wg.Add(1), switching to
> wg.Wait() in the main goroutine. At this point, a connection has been
> accepted, but the WaitGroup counter has not been incremented, so the serve
> function will terminate while silently dropping a network connection (the
> connection will be accepted, but never handled, so will probably eventually
> time out on the client side and leak a file descriptor on the server side).
>
> Is this understanding correct? Furthermore, I suspect that it's impossible
> to implement race-free graceful termination using only sync.WaitGroups. Is
> it actually possible to do so?
>
> Thanks for any correction, and apologies to Dave for picking on his code.
>
> Cheers,
> Tom
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-nuts" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-nuts+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>
-- 

-j

-- 
You received this message because you are subscribed to the Google Groups 
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to golang-nuts+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[go-nuts] Potential race condition in prototypical network server code

2017-08-24 Thread Tom Payne

I'm not singling out Dave Cheney here, I'd just like to check my 
understanding of Go's resource handling and concurrency.

In this blog post a "prototypical network server" is presented:

   https://dave.cheney.net/2017/08/20/context-isnt-for-cancellation

Code:

func serve(l net.Listener) error {
var wg sync.WaitGroup
var conn net.Conn
var err error
for {
conn, err = l.Accept()
if err != nil {
break
}
wg.Add(1)
go func(c net.Conn) {
defer wg.Done()
handle(c)
}(conn)
}
wg.Wait()
return err
}

My understanding is that this contains a race condition. Specifically, a 
goroutine switch can occur at the entry to wg.Add(1), switching to 
wg.Wait() in the main goroutine. At this point, a connection has been 
accepted, but the WaitGroup counter has not been incremented, so the serve 
function will terminate while silently dropping a network connection (the 
connection will be accepted, but never handled, so will probably eventually 
time out on the client side and leak a file descriptor on the server side).

Is this understanding correct? Furthermore, I suspect that it's impossible 
to implement race-free graceful termination using only sync.WaitGroups. Is 
it actually possible to do so?

Thanks for any correction, and apologies to Dave for picking on his code.

Cheers,
Tom

-- 
You received this message because you are subscribed to the Google Groups 
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to golang-nuts+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [go-nuts] Potential race condition in prototypical network server code

Re: [go-nuts] Potential race condition in prototypical network server code

Re: [go-nuts] Potential race condition in prototypical network server code

Re: [go-nuts] Potential race condition in prototypical network server code

Re: [go-nuts] Potential race condition in prototypical network server code

Re: [go-nuts] Potential race condition in prototypical network server code

[go-nuts] Potential race condition in prototypical network server code

7 matches

Site Navigation

Mail list logo

Footer information