code review 7235066: net, os, syscall: don't hold ForkLock during blocked op...

os: don't hold ForkLock across opens on Plan 9

If os.OpenFile holds ForkLock on files that block opens,
then threads that simultaneously try to do fork-exec will
get hung up (until the open succeeds). Blocked opens are
common enough on Plan 9 that protecting against fd leaks
into fork-execs means not being able to do fork-execs
properly in the general case. Thus, we forgo taking the
lock.

[akumar, 2013-01-30 13:11:15]

Hello rsc@golang.org, ality@pbrane.org (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go

[rsc, 2013-01-30 16:14:00]

https://codereview.appspot.com/7235066/diff/3/src/pkg/net/ipsock_plan9.go
File src/pkg/net/ipsock_plan9.go (right):

https://codereview.appspot.com/7235066/diff/3/src/pkg/net/ipsock_plan9.go#new...
src/pkg/net/ipsock_plan9.go:154: f, err := os.OpenFile(l.dir+"/listen",
os.O_RDONLY|syscall.O_CANBLOCK, 0)
This is hardly the only open that can block on Plan 9. I think you have to
accept the race and just never hold the ForkLock across an open.

[akumar, 2013-01-30 16:21:46]

Yeah, it can happen from user code as well, and the
trade-off is between leaking fds and being able to
use os/exec properly. The latter seems more important,
indeed.

On 30 January 2013 16:14,  <rsc@golang.org> wrote:
>
> https://codereview.appspot.com/7235066/diff/3/src/pkg/net/ipsock_plan9.go
> File src/pkg/net/ipsock_plan9.go (right):
>
>
https://codereview.appspot.com/7235066/diff/3/src/pkg/net/ipsock_plan9.go#new...
> src/pkg/net/ipsock_plan9.go:154: f, err := os.OpenFile(l.dir+"/listen",
> os.O_RDONLY|syscall.O_CANBLOCK, 0)
> This is hardly the only open that can block on Plan 9. I think you have
> to accept the race and just never hold the ForkLock across an open.
>
> https://codereview.appspot.com/7235066/

[akumar, 2013-01-30 16:44:56]

PTAL.

Also updated the description.

[rsc, 2013-01-30 17:38:55]

LGTM

[rsc, 2013-01-30 17:41:21]

*** Submitted as https://code.google.com/p/go/source/detail?r=0e84a5cf4e81 ***

os: don't hold ForkLock across opens on Plan 9

If os.OpenFile holds ForkLock on files that block opens,
then threads that simultaneously try to do fork-exec will
get hung up (until the open succeeds). Blocked opens are
common enough on Plan 9 that protecting against fd leaks
into fork-execs means not being able to do fork-execs
properly in the general case. Thus, we forgo taking the
lock.

R=rsc, ality
CC=golang-dev
https://codereview.appspot.com/7235066

Committer: Russ Cox <rsc@golang.org>