Fix concurrent extractionCache read/write#4562
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a concurrency bug in the auto-import registry builder where the @types fallback loop could read from extractionCache while worker goroutines were concurrently writing to it, causing a Go concurrent map read/write race during registryBuilder.updateIndexes.
Changes:
- Guard
extractionCachereads in the@typesfallback loop withextractionMuto prevent concurrent map access. - Add a regression test that exercises the fallback path across many packages to ensure the indexing flow does not crash under concurrent extraction.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| internal/ls/autoimport/registry.go | Locks extractionCache reads during @types fallback candidate evaluation to eliminate concurrent map read/write. |
| internal/ls/autoimport/registry_test.go | Adds a test that builds an auto-import index with many packages that require @types fallback, covering the previously racy path. |
| @@ -963,7 +974,10 @@ func (b *registryBuilder) updateIndexes(ctx context.Context, change RegistryChan | |||
|
|
|||
| // For packages whose main extraction yielded nothing, fall back to @types. | |||
| for _, pkg := range typesFallbackCandidates { | |||
| if extractionCache[pkg.realpath] != nil || seen[pkg.typesRealpath] { | |||
| extractionMu.Lock() | |||
There was a problem hiding this comment.
wg.Wait has happened, so there should be no goroutines still writing to this, so I don't think this is right? I think the full race output would be helpful
There was a problem hiding this comment.
Yes, exactly - because it keeps iterating and spawns a new goroutine on each subsequent iteration, and that workers write can race with the parent goroutine still running the loop.
Fixes an issue uncovered in #4380 (comment), #4380 (comment), and #4380 (comment) (cypress); building the auto-import index in
registryBuilder.updateIndexesspawns workers that populate the sharedextractionCachemap. However, reading this map in the@typesfallback loop is unprotected by the lock and can cause a read/write data race.