cloudflare / cloudflare/workerd
Bug: Possible data race on InspectorService::isolates (main thread vs. inspector thread)
- Dominant language
- C++
- Stars
- 8.7k
- Forks
- 739
- Avg merge
- 2d 20h
- Merged PRs (30d)
- 174
Description
# Summary
I was poking around the codebase with the new Mythos model and it flagged what looks like a thread-safety bug in the inspector so I thought it would be good to make an issue for it.
When the inspector is enabled, two threads end up touching the same `isolates` hashmap without a lock, with the main thread adding entries to it as workers register while the inspector thread reads it to answer `/json/list` (and erases dead entries). If a worker registers while the inspector thread is walking the map, the `insert()` can rehash it mid-iteration and lead to UB.
There is a mutex nearby, but it only guards the pointer to the inspector service, not the map itself.
# Where
`src/workerd/server/server.c++`:
* The map: `isolates` around line 1587
* Inspector thread reads/iterates/erases it in `request()` (lines 1450, 1514, 1482, 1551)
* Main thread inserts via `registerIsolate()` (line 1579)
* The mutex that only covers the pointer (line 1390)
# Notes
The inspector is off by default and dev-only and Worker JS can't reach this map. Just looks like it could crash workerd during local debugging.
Haven't reproduced it at runtime. Please let me know if there's something I missed that makes it safe.
Contributor guide
Assessment
This issue has not been assessed yet.