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

os: data race on os.Process.done #3969

Closed
dvyukov opened this issue Aug 17, 2012 · 6 comments
Closed

os: data race on os.Process.done #3969

dvyukov opened this issue Aug 17, 2012 · 6 comments

Comments

@dvyukov
Copy link
Member

@dvyukov dvyukov commented Aug 17, 2012

When running
$ go test -race -run=none -bench=.* -benchtime=.5 math/big
ThreadSanitizer says:

WARNING: DATA RACE
Write by goroutine 27:
  os.(*Process).wait()
      src/pkg/os/exec_unix.go:27 +0x3a6
  os.(*Process).Wait()
      src/pkg/os/doc.go:43 +0x29
  os/exec.(*Cmd).Wait()
      src/pkg/os/exec/exec.go:302 +0x220
  main.func·022()
      src/cmd/go/test.go:628 +0x3b

Previous read by goroutine 7:
  os.(*Process).signal()
      src/pkg/os/exec_unix.go:38 +0x3a
  os.(*Process).Signal()
      src/pkg/os/doc.go:48 +0x40
  os.(*Process).kill()
      src/pkg/os/exec_posix.go:53 +0x57
  os.(*Process).Kill()
      src/pkg/os/doc.go:36 +0x29
  main.(*builder).runTest()
      src/cmd/go/test.go:634 +0x172d
  main.func·004()
      src/cmd/go/build.go:567 +0x4bb
  main.func·005()
      src/cmd/go/build.go:616 +0x118

Goroutine 27 (running) created at:
  main.(*builder).runTest()
      src/cmd/go/test.go:629 +0xd6a
  main.func·004()
      src/cmd/go/build.go:567 +0x4bb
  main.func·005()
      src/cmd/go/build.go:616 +0x118

Goroutine 7 (running) created at:
  main.(*builder).do()
      src/cmd/go/build.go:618 +0x691
  main.runTest()
      src/cmd/go/test.go:368 +0x26df
  main.main()
      src/cmd/go/main.go:141 +0x6da
  runtime.main()
      src/pkg/runtime/proc.c:247 +0x91
@davecheney

This comment has been minimized.

Copy link
Contributor

@davecheney davecheney commented Aug 18, 2012

Comment 1:

http://golang.org/cl/6462081 is a possible fix which uses channels. pkg/os
already imports sync/atomic so an atomic bool solution is an alternative.
@dvyukov

This comment has been minimized.

Copy link
Member Author

@dvyukov dvyukov commented Aug 18, 2012

Comment 2:

I think an atomic variable is a more appropriate solution here. An atomic
flag is what the code inherently needs, trying to mimic it with chan, close
and select looks a bit awkward to me. Note that the race will be there in
any case (Kill may kill another process), we are just trying to make it
MM-conformant.
@davecheney

This comment has been minimized.

Copy link
Contributor

@davecheney davecheney commented Aug 18, 2012

Comment 3:

Thanks for the feedback, I've updated http://golang.org/cl/6462081 to use
sync/atomic.
@robpike

This comment has been minimized.

Copy link
Contributor

@robpike robpike commented Aug 19, 2012

Comment 4:

Labels changed: added priority-soon, removed priority-triage.

Status changed to Accepted.

@davecheney

This comment has been minimized.

Copy link
Contributor

@davecheney davecheney commented Aug 20, 2012

Comment 5:

6462081 looks good. I will submit once the builders are looking happier.

Owner changed to @davecheney.

Status changed to Started.

@davecheney

This comment has been minimized.

Copy link
Contributor

@davecheney davecheney commented Aug 21, 2012

Comment 6:

This issue was closed by revision 122a558.

Status changed to Fixed.

@dvyukov dvyukov added fixed labels Aug 21, 2012
davecheney added a commit that referenced this issue May 11, 2015
««« backport 4fc1c61c3c2c
os: fix data race on Process.done

Fixes #3969.

R=dvyukov, r, alex.brainman, minux.ma
CC=golang-dev
https://golang.org/cl/6462081

»»»
@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.