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

Jump to bottom

x/tools/gopls/internal/lsp/cache: data race in overlayFS methods #58151

Closed
gopherbot opened this issue Jan 30, 2023 · 7 comments
Closed

x/tools/gopls/internal/lsp/cache: data race in overlayFS methods #58151

gopherbot opened this issue Jan 30, 2023 · 7 comments
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
gopls/v0.12.0

Comments

@gopherbot
Copy link

gopherbot commented Jan 30, 2023 •
edited by bcmills 

#!watchflakes
post <- pkg ~ `^golang\.org/x/tools/gopls/` && log ~ `WARNING: DATA RACE` && log ~ `^\s+golang\.org/x/tools/gopls/internal/lsp/cache\.\(\*overlayFS\)`

Issue created automatically to collect these failures.

Example (log):

--- FAIL: TestLinks (9.94s)
    integration_test.go:436: gopls links a.go: exited with code 66, want 0 (gopls links a.go: exit=66 stdout=<<https://pkg.go.dev/
        https://go.dev/cl
        https://blog.go.dev/
        >> stderr=<<==================
        WARNING: DATA RACE
        Write at 0x00c000560090 by main goroutine:
          runtime.mapassign_faststr()
              /tmp/workdir/go/src/runtime/map_faststr.go:203 +0x0
          golang.org/x/tools/gopls/internal/lsp/cache.(*overlayFS).updateOverlays()
...
              /tmp/workdir/gopath/src/golang.org/x/tools/internal/tool/tool.go:92 +0x225
          golang.org/x/tools/gopls/internal/lsp/cmd/test.goplsMain()
              /tmp/workdir/gopath/src/golang.org/x/tools/gopls/internal/lsp/cmd/test/integration_test.go:766 +0xe4
          golang.org/x/tools/gopls/internal/lsp/cmd/test.TestMain()
              /tmp/workdir/gopath/src/golang.org/x/tools/gopls/internal/lsp/cmd/test/integration_test.go:757 +0x55
          main.main()
              _testmain.go:83 +0x317
        ==================
        Found 1 data race(s)
        >>)

— watchflakes
@gopherbot gopherbot added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 30, 2023
@gopherbot
Copy link
Author

Found new dashboard test flakes for:

#!watchflakes
post <- pkg == "golang.org/x/tools/gopls/internal/lsp/cmd/test" && test == "TestLinks"
2023-01-30 20:26 freebsd-amd64-race tools@87d00e63 go@581603cb x/tools/gopls/internal/lsp/cmd/test.TestLinks (log) 
--- FAIL: TestLinks (9.94s)
    integration_test.go:436: gopls links a.go: exited with code 66, want 0 (gopls links a.go: exit=66 stdout=<<https://pkg.go.dev/
        https://go.dev/cl
        https://blog.go.dev/
        >> stderr=<<==================
        WARNING: DATA RACE
        Write at 0x00c000560090 by main goroutine:
          runtime.mapassign_faststr()
              /tmp/workdir/go/src/runtime/map_faststr.go:203 +0x0
          golang.org/x/tools/gopls/internal/lsp/cache.(*overlayFS).updateOverlays()
...
              /tmp/workdir/gopath/src/golang.org/x/tools/internal/tool/tool.go:92 +0x225
          golang.org/x/tools/gopls/internal/lsp/cmd/test.goplsMain()
              /tmp/workdir/gopath/src/golang.org/x/tools/gopls/internal/lsp/cmd/test/integration_test.go:766 +0xe4
          golang.org/x/tools/gopls/internal/lsp/cmd/test.TestMain()
              /tmp/workdir/gopath/src/golang.org/x/tools/gopls/internal/lsp/cmd/test/integration_test.go:757 +0x55
          main.main()
              _testmain.go:83 +0x317
        ==================
        Found 1 data race(s)
        >>)

— watchflakes

@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Jan 30, 2023
@gopherbot gopherbot added this to the Unreleased milestone Jan 30, 2023
@gopherbot
Copy link
Author

Found new dashboard test flakes for:

#!watchflakes
post <- pkg == "golang.org/x/tools/gopls/internal/lsp/cmd/test" && test == "TestLinks"
2023-01-30 20:48 freebsd-amd64-race tools@d1e92d6a go@581603cb x/tools/gopls/internal/lsp/cmd/test.TestLinks (log) 
--- FAIL: TestLinks (23.86s)
    integration_test.go:444: gopls links -json a.go: exited with code 66, want 0 (gopls links -json a.go: exit=66 stdout=<<[
        	{
        		"range": {
        			"start": {
        				"line": 0,
        				"character": 24
        			},
        			"end": {
        				"line": 0,
...
              /tmp/workdir/gopath/src/golang.org/x/tools/internal/tool/tool.go:92 +0x225
          golang.org/x/tools/gopls/internal/lsp/cmd/test.goplsMain()
              /tmp/workdir/gopath/src/golang.org/x/tools/gopls/internal/lsp/cmd/test/integration_test.go:766 +0xe4
          golang.org/x/tools/gopls/internal/lsp/cmd/test.TestMain()
              /tmp/workdir/gopath/src/golang.org/x/tools/gopls/internal/lsp/cmd/test/integration_test.go:757 +0x55
          main.main()
              _testmain.go:83 +0x317
        ==================
        Found 1 data race(s)
        >>)

— watchflakes

@bcmills bcmills changed the title x/tools/gopls/internal/lsp/cmd/test: TestLinks failures x/tools/gopls/internal/lsp/cmd/test: data race in TestLinks Jan 31, 2023
@bcmills bcmills changed the title x/tools/gopls/internal/lsp/cmd/test: data race in TestLinks x/tools/gopls/internal/lsp/cache: data race in (*overlayFS).Overlay Jan 31, 2023
@bcmills bcmills changed the title x/tools/gopls/internal/lsp/cache: data race in (*overlayFS).Overlay x/tools/gopls/internal/lsp/cache: data race in overlayFS methods Jan 31, 2023
This was referenced Jan 31, 2023
Closed
Closed
Closed
Closed
@bcmills
Copy link
Contributor

bcmills commented Jan 31, 2023

(attn @findleyr @adonovan)

@bcmills bcmills modified the milestones: Unreleased, gopls/v0.12.0 Jan 31, 2023
@gopherbot
Copy link
Author

Found new dashboard test flakes for:

#!watchflakes
post <- pkg ~ `^golang\.org/x/tools/gopls/` && `WARNING: DATA RACE` && `^\s+golang\.org/x/tools/gopls/internal/lsp/cache\.\(\*overlayFS\)`
2023-01-30 20:48 freebsd-amd64-race tools@d1e92d6a go@01a5a83c x/tools/gopls/internal/lsp/cmd/test.TestImports (log) 
--- FAIL: TestImports (6.86s)
    integration_test.go:402: gopls imports -diff a.go: exited with code 66, want 0 (gopls imports -diff a.go: exit=66 stdout=<<--- ./a.go.orig
        +++ ./a.go
        @@ -1,4 +1,6 @@
         package a
        +
        +import "fmt"
         func _() {
         	fmt.Println()
         }
...
              /tmp/workdir/gopath/src/golang.org/x/tools/internal/tool/tool.go:92 +0x233
          golang.org/x/tools/gopls/internal/lsp/cmd/test.goplsMain()
              /tmp/workdir/gopath/src/golang.org/x/tools/gopls/internal/lsp/cmd/test/integration_test.go:766 +0xdd
          golang.org/x/tools/gopls/internal/lsp/cmd/test.TestMain()
              /tmp/workdir/gopath/src/golang.org/x/tools/gopls/internal/lsp/cmd/test/integration_test.go:757 +0x55
          main.main()
              _testmain.go:83 +0x324
        ==================
        Found 1 data race(s)
        >>)
2023-01-30 20:57 linux-amd64-longtest-race tools@26831282 go@2ab0e046 x/tools/gopls/internal/regtest/workspace.TestNewModule_Issue38207 (log) 
==================
WARNING: DATA RACE
Read at 0x00c009bd9198 by goroutine 90361:
  golang.org/x/tools/gopls/internal/lsp/cache.(*overlayFS).Overlays()
      /workdir/gopath/src/golang.org/x/tools/gopls/internal/lsp/cache/fs_overlay.go:34 +0xda4
  golang.org/x/tools/gopls/internal/lsp.(*Server).diagnose()
      /workdir/gopath/src/golang.org/x/tools/gopls/internal/lsp/diagnostics.go:352 +0xc2c
  golang.org/x/tools/gopls/internal/lsp.(*Server).diagnoseSnapshot()
      /workdir/gopath/src/golang.org/x/tools/gopls/internal/lsp/diagnostics.go:167 +0x2ea
  golang.org/x/tools/gopls/internal/lsp.(*Server).diagnoseSnapshots.func1()
...

[Trace - 21:21:53.335 PM] Received notification '$/progress'.
Params: {"token":"475964039857791957","value":{"kind":"end","message":"Done."}}


#### End Gopls Test Logs for "TestNewModule_Issue38207/forwarded"
--- FAIL: TestNewModule_Issue38207 (1.25s)
    --- FAIL: TestNewModule_Issue38207/forwarded (0.44s)
        testing.go:1446: race detected during execution of test
    testing.go:1446: race detected during execution of test
2023-01-30 20:57 linux-amd64-longtest-race tools@26831282 go@2ab0e046 x/tools/gopls/internal/regtest/workspace.TestBrokenWorkspace_WrongModulePath (log) 
==================
WARNING: DATA RACE
Read at 0x00c0002960c0 by goroutine 318:
  golang.org/x/tools/gopls/internal/lsp/cache.(*overlayFS).Overlays()
      /workdir/gopath/src/golang.org/x/tools/gopls/internal/lsp/cache/fs_overlay.go:33 +0xcb1
  golang.org/x/tools/gopls/internal/lsp.(*Server).diagnose()
      /workdir/gopath/src/golang.org/x/tools/gopls/internal/lsp/diagnostics.go:352 +0xc2c
  golang.org/x/tools/gopls/internal/lsp.(*Server).diagnoseSnapshot()
      /workdir/gopath/src/golang.org/x/tools/gopls/internal/lsp/diagnostics.go:167 +0x2ea
  golang.org/x/tools/gopls/internal/lsp.(*Server).diagnoseSnapshots.func1()
...

[Trace - 21:20:12.086 PM] Received notification '$/progress'.
Params: {"token":"5715915864827999571","value":{"kind":"end","message":"Done."}}


#### End Gopls Test Logs for "TestBrokenWorkspace_WrongModulePath/default"
--- FAIL: TestBrokenWorkspace_WrongModulePath (7.39s)
    --- FAIL: TestBrokenWorkspace_WrongModulePath/default (2.88s)
        testing.go:1446: race detected during execution of test
    testing.go:1446: race detected during execution of test

— watchflakes

@findleyr
Copy link
Contributor

@adonovan fixed this (for me) in https://go.dev/cl/464057. Thanks!

@gopherbot
Copy link
Author

Found new dashboard test flakes for:

#!watchflakes
post <- pkg ~ `^golang\.org/x/tools/gopls/` && `WARNING: DATA RACE` && `^\s+golang\.org/x/tools/gopls/internal/lsp/cache\.\(\*overlayFS\)`
2023-01-30 19:34 linux-amd64-longtest-race tools@ae242ec3 go@1c4cfb92 x/tools/gopls/internal/lsp/cmd/test.TestImports (log) 
--- FAIL: TestImports (5.39s)
    integration_test.go:402: gopls imports -diff a.go: exited with code 66, want 0 (gopls imports -diff a.go: exit=66 stdout=<<--- ./a.go.orig
        +++ ./a.go
        @@ -1,4 +1,6 @@
         package a
        +
        +import "fmt"
         func _() {
         	fmt.Println()
         }
...
              /workdir/gopath/src/golang.org/x/tools/internal/tool/tool.go:92 +0x233
          golang.org/x/tools/gopls/internal/lsp/cmd/test.goplsMain()
              /workdir/gopath/src/golang.org/x/tools/gopls/internal/lsp/cmd/test/integration_test.go:766 +0xd9
          golang.org/x/tools/gopls/internal/lsp/cmd/test.TestMain()
              /workdir/gopath/src/golang.org/x/tools/gopls/internal/lsp/cmd/test/integration_test.go:757 +0x55
          main.main()
              _testmain.go:83 +0x32f
        ==================
        Found 1 data race(s)
        >>)
2023-01-30 19:34 linux-amd64-longtest-race tools@ae242ec3 go@57f9ed5f x/tools/gopls/internal/regtest/completion.TestDefinition (log) 
==================
WARNING: DATA RACE
Read at 0x00c003fffb00 by goroutine 33902:
  golang.org/x/tools/gopls/internal/lsp/cache.(*overlayFS).Overlays()
      /workdir/gopath/src/golang.org/x/tools/gopls/internal/lsp/cache/fs_overlay.go:33 +0xcb1
  golang.org/x/tools/gopls/internal/lsp.(*Server).diagnose()
      /workdir/gopath/src/golang.org/x/tools/gopls/internal/lsp/diagnostics.go:352 +0xc2c
  golang.org/x/tools/gopls/internal/lsp.(*Server).diagnoseSnapshot()
      /workdir/gopath/src/golang.org/x/tools/gopls/internal/lsp/diagnostics.go:167 +0x2ea
  golang.org/x/tools/gopls/internal/lsp.(*Server).diagnoseSnapshots.func1()
...

[Trace - 19:42:44.695 PM] Received response 'textDocument/completion - (18)' in 27ms.
Result: {"isIncomplete":true,"items":[{"label":"TestMain","kind":3,"documentation":"complete the function name","preselect":true,"sortText":"00000","filterText":"TestMain","insertTextFormat":2,"textEdit":{"range":{"start":{"line":1,"character":5},"end":{"line":1,"character":7}},"newText":"TestMain"}},{"label":"Test","kind":3,"documentation":"complete the function name","sortText":"00001","filterText":"Test","insertTextFormat":2,"textEdit":{"range":{"start":{"line":1,"character":5},"end":{"line":1,"character":7}},"newText":"Test"}}]}


#### End Gopls Test Logs for "TestDefinition/forwarded"
--- FAIL: TestDefinition (1.89s)
    --- FAIL: TestDefinition/forwarded (0.83s)
        testing.go:1446: race detected during execution of test
    testing.go:1446: race detected during execution of test
2023-01-30 20:48 linux-amd64-longtest-race tools@d1e92d6a go@2ab0e046 x/tools/gopls/internal/regtest/modfile.TestUnknownRevision (log) 
==================
WARNING: DATA RACE
Read at 0x00c000c7a1e0 by goroutine 20174:
  golang.org/x/tools/gopls/internal/lsp/cache.(*overlayFS).Overlays()
      /workdir/gopath/src/golang.org/x/tools/gopls/internal/lsp/cache/fs_overlay.go:33 +0xcb1
  golang.org/x/tools/gopls/internal/lsp.(*Server).diagnose()
      /workdir/gopath/src/golang.org/x/tools/gopls/internal/lsp/diagnostics.go:352 +0xc2c
  golang.org/x/tools/gopls/internal/lsp.(*Server).diagnoseSnapshot()
      /workdir/gopath/src/golang.org/x/tools/gopls/internal/lsp/diagnostics.go:167 +0x2ea
  golang.org/x/tools/gopls/internal/lsp.(*Server).diagnoseSnapshots.func1()
...

#### End Gopls Test Logs for "TestUnknownRevision/good/nested/default"
--- FAIL: TestUnknownRevision (10.20s)
    --- FAIL: TestUnknownRevision/good (5.33s)
        --- FAIL: TestUnknownRevision/good/nested (2.99s)
            --- FAIL: TestUnknownRevision/good/nested/default (1.02s)
                testing.go:1446: race detected during execution of test
            testing.go:1446: race detected during execution of test
        testing.go:1446: race detected during execution of test
    testing.go:1446: race detected during execution of test
2023-01-30 20:48 linux-amd64-race tools@d1e92d6a go@581603cb x/tools/gopls/internal/regtest/misc.TestRenamePackage_InternalPackage (log) 
serve.go:434: debug server listening at http://localhost:45529
serve.go:434: debug server listening at http://localhost:44669
==================
WARNING: DATA RACE
Write at 0x00c0016a43f0 by goroutine 401:
  runtime.mapassign_faststr()
      /workdir/go/src/runtime/map_faststr.go:203 +0x0
  golang.org/x/tools/gopls/internal/lsp/cache.(*overlayFS).updateOverlays()
      /workdir/gopath/src/golang.org/x/tools/gopls/internal/lsp/cache/session.go:708 +0x76f
  golang.org/x/tools/gopls/internal/lsp/cache.(*Session).DidModifyFiles()
...

[Trace - 20:53:41.841 PM] Sending notification 'workspace/didChangeWatchedFiles'.
Params: {"changes":[{"uri":"file:///workdir/tmp/gopls-regtest-3388992410/TestRenamePackage_InternalPackage/default/work/lib1/a.go","type":1},{"uri":"file:///workdir/tmp/gopls-regtest-3388992410/TestRenamePackage_InternalPackage/default/work/lib1/internal/utils/a.go","type":1},{"uri":"file:///workdir/tmp/gopls-regtest-3388992410/TestRenamePackage_InternalPackage/default/work/lib/a.go","type":3},{"uri":"file:///workdir/tmp/gopls-regtest-3388992410/TestRenamePackage_InternalPackage/default/work/lib/internal/utils/a.go","type":3}]}


#### End Gopls Test Logs for "TestRenamePackage_InternalPackage/default"
--- FAIL: TestRenamePackage_InternalPackage (1.81s)
    --- FAIL: TestRenamePackage_InternalPackage/default (1.81s)
        testing.go:1312: race detected during execution of test
    testing.go:1312: race detected during execution of test
2023-01-30 20:48 windows-amd64-race tools@d1e92d6a go@2ab0e046 x/tools/gopls/internal/lsp/cmd/test.TestImports (log) 
--- FAIL: TestImports (14.58s)
    integration_test.go:408: gopls imports -write a.go: exited with code 66, want 0 (gopls imports -write a.go: exit=66 stdout=<<>> stderr=<<==================
        WARNING: DATA RACE
        Read at 0x00c00023a930 by goroutine 109:
          golang.org/x/tools/gopls/internal/lsp/cache.(*overlayFS).Overlays()
              C:/workdir/gopath/src/golang.org/x/tools/gopls/internal/lsp/cache/fs_overlay.go:33 +0xcb1
          golang.org/x/tools/gopls/internal/lsp.(*Server).diagnose()
              C:/workdir/gopath/src/golang.org/x/tools/gopls/internal/lsp/diagnostics.go:352 +0xc2c
          golang.org/x/tools/gopls/internal/lsp.(*Server).diagnoseDetached()
              C:/workdir/gopath/src/golang.org/x/tools/gopls/internal/lsp/diagnostics.go:127 +0x8a
...
              C:/workdir/gopath/src/golang.org/x/tools/internal/tool/tool.go:92 +0x233
          golang.org/x/tools/gopls/internal/lsp/cmd/test.goplsMain()
              C:/workdir/gopath/src/golang.org/x/tools/gopls/internal/lsp/cmd/test/integration_test.go:766 +0xd9
          golang.org/x/tools/gopls/internal/lsp/cmd/test.TestMain()
              C:/workdir/gopath/src/golang.org/x/tools/gopls/internal/lsp/cmd/test/integration_test.go:757 +0x55
          main.main()
              _testmain.go:83 +0x32f
        ==================
        Found 3 data race(s)
        >>)

— watchflakes

@adonovan
Copy link
Member

adonovan commented Feb 2, 2023

These reports are all before or around the time the bug was plausibly fixed, so I don't plan to reopen this.

@golang golang locked and limited conversation to collaborators Feb 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker Tools This label describes issues relating to any tools in the x/tools repository.
Projects
Test Flakes
Status: Done
Milestone
gopls/v0.12.0
Development

No branches or pull requests
4 participants
@bcmills @adonovan @gopherbot @findleyr