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

go/token: FileSet.PositionFor not synchronized. #16548

Closed
cznic opened this issue Jul 30, 2016 · 2 comments
Closed

go/token: FileSet.PositionFor not synchronized. #16548

cznic opened this issue Jul 30, 2016 · 2 comments
Milestone

Comments

@cznic
Copy link
Contributor

cznic commented Jul 30, 2016

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

go version go1.7rc3 linux/amd64

  • What operating system and processor architecture are you using (go env)?
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/jnml"
GORACE=""
GOROOT="/home/jnml/go"
GOTOOLDIR="/home/jnml/go/pkg/tool/linux_amd64"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build215349180=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
  • What did you do?

Copy this code to somewhere in your $GOPATH and name it, for example, race_test.go.

package race

import (
    "go/token"
    "testing"
)

const N = 1e3

var (
    fset = token.NewFileSet()
    file = fset.AddFile("", -1, N)
)

func TestRace(t *testing.T) {
    ch := make(chan int, 2)

    go func() {
        for i := 0; i < N; i++ {
            file.AddLine(i)
        }
        ch <- 1
    }()

    go func() {
        pos := file.Pos(0)
        for i := 0; i < N; i++ {
            fset.PositionFor(pos, false)
        }
        ch <- 1
    }()

    <-ch
    <-ch
}

cd to that place and invoke

$ go test -race
  • What did you expect to see?
jnml@4670:~/src/github.com/cznic/token_issue$ go test -race
PASS
ok      github.com/cznic/token_issue    1.005s
jnml@4670:~/src/github.com/cznic/token_issue$ 
  • What did you see instead?
jnml@4670:~/src/github.com/cznic/token_issue$ go test -race
==================
WARNING: DATA RACE
Write at 0x00c4200660e8 by goroutine 7:
  go/token.(*File).AddLine()
      /home/jnml/go/src/go/token/position.go:132 +0x15b
  github.com/cznic/token_issue.TestRace.func1()
      /home/jnml/src/github.com/cznic/token_issue/race_test.go:20 +0x6a

Previous read at 0x00c4200660e8 by goroutine 8:
  go/token.(*File).unpack()
      /home/jnml/go/src/go/token/position.go:271 +0x83
  go/token.(*File).position()
      /home/jnml/go/src/go/token/position.go:290 +0x9d
  go/token.(*FileSet).PositionFor()
      /home/jnml/go/src/go/token/position.go:449 +0xc7
  github.com/cznic/token_issue.TestRace.func2()
      /home/jnml/src/github.com/cznic/token_issue/race_test.go:28 +0xa3

Goroutine 7 (running) created at:
  github.com/cznic/token_issue.TestRace()
      /home/jnml/src/github.com/cznic/token_issue/race_test.go:23 +0x6e
  testing.tRunner()
      /home/jnml/go/src/testing/testing.go:610 +0xc9

Goroutine 8 (running) created at:
  github.com/cznic/token_issue.TestRace()
      /home/jnml/src/github.com/cznic/token_issue/race_test.go:31 +0x90
  testing.tRunner()
      /home/jnml/go/src/testing/testing.go:610 +0xc9
==================
PASS
Found 1 data race(s)
exit status 66
FAIL    github.com/cznic/token_issue    1.021s
jnml@4670:~/src/github.com/cznic/token_issue$ 
  • Discussion

Methods of FileSet are documented to be safe for concurrent use by multiple goroutines, so FileSet is protected by a mutex and all its methods use it to prevent concurrent mutations. All methods of File that mutate the respective FileSet, including AddLine, do also lock its mutex, but that does not help when PositionFor is invoked concurrently and reads without synchronization what AddLine mutates.

I think a mutex [r]lock is missing in PositionFor.

@quentinmit quentinmit added this to the Go1.8 milestone Aug 1, 2016
@quentinmit
Copy link
Contributor

NIce find. Feel free to send a CL on Gerrit.

@gopherbot
Copy link

CL https://golang.org/cl/25345 mentions this issue.

@golang golang locked and limited conversation to collaborators Aug 16, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants