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

internal/poll: data race in (*FD).SetBlocking #24481

Closed
twmb opened this issue Mar 21, 2018 · 5 comments
Closed

internal/poll: data race in (*FD).SetBlocking #24481

twmb opened this issue Mar 21, 2018 · 5 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@twmb
Copy link
Contributor

twmb commented Mar 21, 2018

What version of Go are you using (go version)?

go version devel +7974f0815e Mon Mar 19 21:51:23 2018 +0000 linux/amd64

This is new in 1.10.

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOCACHE="/home/twmb/.cache/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/twmb/go"
GORACE=""
GOROOT="/home/twmb/go/go"
GOTMPDIR=""
GOTOOLDIR="/home/twmb/go/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build478075614=/tmp/go-build -gno-record-gcc-switches"

What did you do?

For net:
https://play.golang.org/p/p2p0vliZwJB
For os:
https://play.golang.org/p/st7Npi4-2oQ

What did you expect to see?

With go run -race on that file, no leak.

What did you see instead?

==================
WARNING: DATA RACE
Write at 0x00c42013e7af by goroutine 64:
  internal/poll.(*FD).SetBlocking()
      /home/twmb/go/go/src/internal/poll/fd_unix.go:121 +0x8f
  net.(*netFD).dup()
      /home/twmb/go/go/src/net/fd_unix.go:316 +0x7d
  net.(*conn).File()
      /home/twmb/go/go/src/net/net.go:294 +0x4d
  main.main.func2()
      /home/twmb/testing/junk.go:29 +0x3a

Previous write at 0x00c42013e7af by goroutine 66:
  internal/poll.(*FD).SetBlocking()
      /home/twmb/go/go/src/internal/poll/fd_unix.go:121 +0x8f
  net.(*netFD).dup()
      /home/twmb/go/go/src/net/fd_unix.go:316 +0x7d
  net.(*conn).File()
      /home/twmb/go/go/src/net/net.go:294 +0x4d
  main.main.func3()
      /home/twmb/testing/junk.go:34 +0x3a

Goroutine 64 (running) created at:
  main.main()
      /home/twmb/testing/junk.go:28 +0x13f

Goroutine 66 (running) created at:
  main.main()
      /home/twmb/testing/junk.go:33 +0x161
==================

For os:

starting
==================
WARNING: DATA RACE
Write at 0x00c42009416f by goroutine 10:
  internal/poll.(*FD).SetBlocking()
      /home/twmb/go/go/src/internal/poll/fd_unix.go:121 +0x8f
  os.(*File).Fd()
      /home/twmb/go/go/src/os/file_unix.go:69 +0xce

Previous write at 0x00c42009416f by goroutine 11:
  internal/poll.(*FD).SetBlocking()
      /home/twmb/go/go/src/internal/poll/fd_unix.go:121 +0x8f
  os.(*File).Fd()
      /home/twmb/go/go/src/os/file_unix.go:69 +0xce

Goroutine 10 (running) created at:
  main.main()
      /home/twmb/testing/junk.go:18 +0xa3

Goroutine 11 (finished) created at:
  main.main()
      /home/twmb/testing/junk.go:19 +0xc5
==================
done

0f3ab149ec4 introduced SetBlocking on file descriptors that can be pollable. SetBlocking sets isBlocking in a struct. SetBlocking is not called often, but the call itself is unprotected when it is. Documentation does not suggest that the calls themselves should be protected.

@bcmills bcmills changed the title net: concurrent File calls are racy; os: concurrent Fd calls on non-blocking files are racy internal/poll: data race in (*FD).SetBlocking Mar 21, 2018
@bcmills
Copy link
Contributor

bcmills commented Mar 21, 2018

(CC @ianlancetaylor)

@andybons andybons added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 21, 2018
@andybons andybons added this to the Go1.11 milestone Mar 21, 2018
@ianlancetaylor
Copy link
Contributor

I believe the fix mentioned in #24483 would fix this one too. I am still skeptical as to whether this is a problem that real programs will encounter.

@ianlancetaylor
Copy link
Contributor

This data race in the net package was removed as a side-effect of fixing #24942.

@gopherbot
Copy link

Change https://golang.org/cl/119955 mentions this issue: os, net: avoid races between dup, set-blocking-mode, and closing

@gopherbot
Copy link

Change https://golang.org/cl/123176 mentions this issue: internal/poll: don't take read lock in SetBlocking

gopherbot pushed a commit that referenced this issue Jul 11, 2018
Taking a read lock in SetBlocking could cause SetBlocking to block
waiting for a Read in another goroutine to complete. Since SetBlocking
is called by os.(*File).Fd, that could lead to deadlock if the
goroutine calling Fd is going to use it to unblock the Read.
Use an atomic store instead.

Updates #24481

Change-Id: I79413328e06ddf28b6d5b8af7a0e29d5b4e1e6ff
Reviewed-on: https://go-review.googlesource.com/123176
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators Jul 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

5 participants