Skip to content

Fix concurrent map access on task index map in flow tasks#209

Open
damilolaedwards wants to merge 1 commit into
ethpandaops:masterfrom
damilolaedwards:fix-concurrent-taskidxmap-race
Open

Fix concurrent map access on task index map in flow tasks#209
damilolaedwards wants to merge 1 commit into
ethpandaops:masterfrom
damilolaedwards:fix-concurrent-taskidxmap-race

Conversation

@damilolaedwards

Copy link
Copy Markdown

Problem

The concurrent (run_tasks_concurrent) and matrix (run_task_matrix) flow tasks wrote the task index map inside the child spawn loop, while each child's watcher goroutine read the same map. That is a concurrent map read and write.

The Go runtime detects this and raises a fatal error ("concurrent map read and map write"), which is not a panic and cannot be recovered by the scheduler's per-task recover. When it triggers, the whole assertoor process dies, aborting every test that is currently running, not just the offending one.

The read happens in watchChildTask (via the debug log of the task index), and the watcher goroutines are started from ExecuteTask while the spawn loop is still writing entries for later children, so a fast or skipped child can hit the read before the map is fully populated.

Fix

Populate the task index map fully in a dedicated pass before spawning any child goroutine. The mapping is known up front and never changes after that, so there is no reason to write it concurrently with the readers.

Tests

Each package gets a race regression test that drives the real Execute with many children and a stub scheduler whose ExecuteTask starts the watcher goroutine exactly as the real scheduler does. The tests pass under -race with the fix and fail without it.

go build ./..., go vet, and go test -race on both packages pass.

The concurrent and matrix flow tasks wrote the task index map inside the
child spawn loop while each child's watcher goroutine read it. That is a
concurrent map read and write, which the Go runtime turns into a fatal
error that cannot be recovered, taking down the whole process and every
running test.

Populate the index map fully before spawning any child goroutine, since
the mapping is known up front and never changes after that. Add race
regression tests that drive the executors with many children and fail
under -race without the fix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant