search

StyraInc / regal

Regal is a linter for Rego, with the goal of making your Rego magnificent!
https://docs.styra.com/regal
Apache License 2.0
237 stars 30 forks source link

Completions: "fatal error: concurrent map iteration and map write" when trying completion #750

Closed anderseknert closed 1 month ago

anderseknert commented 1 month ago

Sadly, I'm not 100% sure what I did to cause this, as I was frantically testing different things trying to break completions, and while I succeeded in that, I'm not sure exactly what caused the breakage. But it looks like we need a lock somewhere we don't.

fatal error: concurrent map iteration and map write

goroutine 34 [running]:
github.com/styrainc/regal/internal/lsp/completions/providers.(*RuleFromImportedPackageRefs).Run(0x140021e9a70?, 0x140003d5420, {{{0x1400e975ec0, 0x31}}, {0x4, 0xa}, {0x1, {0x0, 0x0}}}, 0x1400160c3b0?)
    /Users/anderseknert/git/styra/regal/internal/lsp/completions/providers/rulerefs.go:45 +0x238
github.com/styrainc/regal/internal/lsp/completions.(*Manager).Run(0x140003f62a0, {{{0x1400e975ec0, 0x31}}, {0x4, 0xa}, {0x1, {0x0, 0x0}}}, 0x1400160c3b0)
    /Users/anderseknert/git/styra/regal/internal/lsp/completions/manager.go:44 +0xd0
github.com/styrainc/regal/internal/lsp.(*LanguageServer).handleTextDocumentCompletion(0x1400039ab00, {0x105696840?, 0x1400160c1a0?}, 0x105696840?, 0x140198c7b00)
    /Users/anderseknert/git/styra/regal/internal/lsp/server.go:690 +0x118
github.com/styrainc/regal/internal/lsp.(*LanguageServer).Handle(0x1400039ab00, {0x10579f988, 0x140003b1c20}, 0x140003aa990, 0x140198c7b00)
    /Users/anderseknert/git/styra/regal/internal/lsp/server.go:143 +0x4f4
github.com/sourcegraph/jsonrpc2.(*HandlerWithErrorConfigurer).Handle(0x1400039ec20, {0x10579f988, 0x140003b1c20}, 0x140003aa990, 0x140198c7b00)
    /Users/anderseknert/go/pkg/mod/github.com/sourcegraph/jsonrpc2@v0.2.0/handler_with_error.go:21 +0x4c
github.com/sourcegraph/jsonrpc2.(*Conn).readMessages(0x140003aa990, {0x10579f988, 0x140003b1c20})
    /Users/anderseknert/go/pkg/mod/github.com/sourcegraph/jsonrpc2@v0.2.0/conn.go:205 +0x2a4
created by github.com/sourcegraph/jsonrpc2.NewConn in goroutine 1
    /Users/anderseknert/go/pkg/mod/github.com/sourcegraph/jsonrpc2@v0.2.0/conn.go:62 +0x1d0

goroutine 1 [select, 30 minutes]:
github.com/styrainc/regal/cmd.init.2.func1({0x140003b2200?, 0x1050c10f2?, 0x0?})
    /Users/anderseknert/git/styra/regal/cmd/languageserver.go:51 +0x36c
github.com/styrainc/regal/cmd.init.2.wrapProfiling.func2(0x140003e2608, {0x105e70260, 0x0, 0x0})
    /Users/anderseknert/git/styra/regal/cmd/profiling.go:16 +0x6b4
github.com/spf13/cobra.(*Command).execute(0x140003e2608, {0x105e70260, 0x0, 0x0})
    /Users/anderseknert/go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:983 +0x840
github.com/spf13/cobra.(*Command).ExecuteC(0x105df6820)
    /Users/anderseknert/go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:1115 +0x344
github.com/spf13/cobra.(*Command).Execute(0x14000074738?)
    /Users/anderseknert/go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:1039 +0x1c
main.main()
    /Users/anderseknert/git/styra/regal/main.go:16 +0x40

goroutine 35 [runnable]:
embed.FS.readDir({0x1057b8fe0?}, {0x1055a2db0, 0x1})
    /opt/homebrew/Cellar/go/1.22.2/libexec/src/embed/embed.go:284 +0x150
embed.FS.Open({0x105ddda20?}, {0x1055a2db0, 0x1})
    /opt/homebrew/Cellar/go/1.22.2/libexec/src/embed/embed.go:311 +0x140
io/fs.Stat({0x105798b68?, 0x1057b8fe0?}, {0x1055a2db0?, 0x1?})
    /opt/homebrew/Cellar/go/1.22.2/libexec/src/io/fs/stat.go:25 +0x70
io/fs.WalkDir({0x105798b68, 0x1057b8fe0}, {0x1055a2db0, 0x1}, 0x140019fc080)
    /opt/homebrew/Cellar/go/1.22.2/libexec/src/io/fs/walk.go:118 +0x30
github.com/open-policy-agent/opa/bundle.(*dirLoaderFS).NextFile(0x1401d0060c0)
    /Users/anderseknert/go/pkg/mod/github.com/open-policy-agent/opa@v0.64.1/bundle/filefs.go:104 +0xe8
github.com/open-policy-agent/opa/bundle.preProcessBundle({0x10579f8e0, 0x1401d0060c0}, 0x1, 0x40000001)
    /Users/anderseknert/go/pkg/mod/github.com/open-policy-agent/opa@v0.64.1/bundle/bundle.go:1625 +0xa0
github.com/open-policy-agent/opa/bundle.(*Reader).Read(_)
    /Users/anderseknert/go/pkg/mod/github.com/open-policy-agent/opa@v0.64.1/bundle/bundle.go:577 +0x88
github.com/styrainc/regal/internal/io.LoadRegalBundleFS({_, _})
    /Users/anderseknert/git/styra/regal/internal/io/io.go:32 +0x12c
github.com/styrainc/regal/internal/io.MustLoadRegalBundleFS({_, _})
    /Users/anderseknert/git/styra/regal/internal/io/io.go:47 +0x5c
github.com/styrainc/regal/pkg/linter.NewLinter(...)
    /Users/anderseknert/git/styra/regal/pkg/linter/linter.go:77
github.com/styrainc/regal/internal/lsp.updateFileDiagnostics({0x10579f988, 0x140003b1c20}, 0x140003d5420, 0x14000ba4000, {0x1400e975dc0, 0x31}, {0x140001251a0, 0x2a})
    /Users/anderseknert/git/styra/regal/internal/lsp/lint.go:139 +0x1b0
github.com/styrainc/regal/internal/lsp.(*LanguageServer).StartDiagnosticsWorker(0x1400039ab00, {0x10579f988, 0x140003b1c20})
    /Users/anderseknert/git/styra/regal/internal/lsp/server.go:225 +0x214
created by github.com/styrainc/regal/cmd.init.2.func1 in goroutine 1
    /Users/anderseknert/git/styra/regal/cmd/languageserver.go:43 +0x1c4

goroutine 36 [select]:
github.com/styrainc/regal/internal/lsp.(*LanguageServer).StartHoverWorker(0x1400039ab00, {0x10579f988, 0x140003b1c20})
    /Users/anderseknert/git/styra/regal/internal/lsp/server.go:261 +0x90
created by github.com/styrainc/regal/cmd.init.2.func1 in goroutine 1
    /Users/anderseknert/git/styra/regal/cmd/languageserver.go:44 +0x21c

goroutine 37 [select, 30 minutes]:
github.com/styrainc/regal/internal/lsp.(*LanguageServer).StartCommandWorker(0x1400039ab00, {0x10579f988, 0x140003b1c20})
    /Users/anderseknert/git/styra/regal/internal/lsp/server.go:328 +0x8c
created by github.com/styrainc/regal/cmd.init.2.func1 in goroutine 1
    /Users/anderseknert/git/styra/regal/cmd/languageserver.go:45 +0x274

goroutine 38 [select, 30 minutes]:
github.com/styrainc/regal/internal/lsp.(*LanguageServer).StartConfigWorker(0x1400039ab00, {0x10579f988, 0x140003b1c20})
    /Users/anderseknert/git/styra/regal/internal/lsp/server.go:282 +0x120
created by github.com/styrainc/regal/cmd.init.2.func1 in goroutine 1
    /Users/anderseknert/git/styra/regal/cmd/languageserver.go:46 +0x2cc

goroutine 40 [syscall, 30 minutes]:
os/signal.signal_recv()
    /opt/homebrew/Cellar/go/1.22.2/libexec/src/runtime/sigqueue.go:149 +0x2c
os/signal.loop()
    /opt/homebrew/Cellar/go/1.22.2/libexec/src/os/signal/signal_unix.go:23 +0x1c
created by os/signal.Notify.func1.1 in goroutine 1
    /opt/homebrew/Cellar/go/1.22.2/libexec/src/os/signal/signal.go:151 +0x28

goroutine 6 [syscall, 30 minutes]:
syscall.syscall6(0x14000409c38?, 0x1048e930c?, 0x104de51c0?, 0x8?, 0x14000409c08?, 0x0?, 0x0?)
    /opt/homebrew/Cellar/go/1.22.2/libexec/src/runtime/sys_darwin.go:45 +0x68
golang.org/x/sys/unix.kevent(0x14000409d78?, 0x1?, 0x0?, 0x105e28750?, 0x14000409c88?, 0x1048e006c?)
    /Users/anderseknert/go/pkg/mod/golang.org/x/sys@v0.19.0/unix/zsyscall_darwin_arm64.go:275 +0x54
golang.org/x/sys/unix.Kevent(0x14000409c88?, {0x0?, 0x0?, 0x104de51d0?}, {0x14000409d80?, 0x1060cc130?, 0x14000409c98?}, 0x105020ba8?)
    /Users/anderseknert/go/pkg/mod/golang.org/x/sys@v0.19.0/unix/syscall_bsd.go:397 +0x40
github.com/fsnotify/fsnotify.(*Watcher).read(0x14000409cd8?, {0x14000409d80, 0x0?, 0xa})
    /Users/anderseknert/go/pkg/mod/github.com/fsnotify/fsnotify@v1.7.0/backend_kqueue.go:777 +0x44
github.com/fsnotify/fsnotify.(*Watcher).readEvents(0x140000c8000)
    /Users/anderseknert/go/pkg/mod/github.com/fsnotify/fsnotify@v1.7.0/backend_kqueue.go:547 +0x9c
created by github.com/fsnotify/fsnotify.NewBufferedWatcher in goroutine 38
    /Users/anderseknert/go/pkg/mod/github.com/fsnotify/fsnotify@v1.7.0/backend_kqueue.go:184 +0x1fc

goroutine 7 [select, 30 minutes]:
github.com/styrainc/regal/internal/lsp/config.(*Watcher).loop(0x140003a0740, {0x10579f988, 0x140003b1c20})
    /Users/anderseknert/git/styra/regal/internal/lsp/config/watcher.go:63 +0xd8
github.com/styrainc/regal/internal/lsp/config.(*Watcher).Start.func1()
    /Users/anderseknert/git/styra/regal/internal/lsp/config/watcher.go:55 +0x28
created by github.com/styrainc/regal/internal/lsp/config.(*Watcher).Start in goroutine 38
    /Users/anderseknert/git/styra/regal/internal/lsp/config/watcher.go:54 +0x130
[Error - 13:40:28] Server process exited with code 2.
[Error - 13:40:28] Connection to server got closed. Server will not be restarted.
[Error - 13:40:28] Delivering pending changes failed
TypeError: this.task is not a function
    at /Users/anderseknert/.vscode/extensions/tsandall.opa-0.15.0/node_modules/vscode-languageclient/lib/common/utils/async.js:28:35
charlieegan3 commented 1 month ago

I can't seem to replicate this, but here is my theory. This code safely gets all the refs, however this function (GetAllFileRefs) only returns a reference to the map.

for file, refs := range c.GetAllFileRefs()

Then while we loop over it, there's an update to the underlying contents creating this issue. I think that the same issue could have happened in GetAllBuiltInPositions, GetAllModules & GetAllFiles, as well as GetAllFileRefs.

I think that the fix here is to, for these functions, return a copy of the underlying map instead.

charlieegan3 commented 1 month ago

https://github.com/StyraInc/regal/pull/760 would be a fix if this we agree on this being the cause.