Skip to content

Release resource registry read lock before running handlers#281

Open
damilolaedwards wants to merge 1 commit into
ethpandaops:masterfrom
damilolaedwards:fix/resource-registry-read-deadlock
Open

Release resource registry read lock before running handlers#281
damilolaedwards wants to merge 1 commit into
ethpandaops:masterfrom
damilolaedwards:fix/resource-registry-read-deadlock

Conversation

@damilolaedwards

Copy link
Copy Markdown

Problem

registry.Read held the registry read lock while invoking a resource handler. Some handlers list other resources while running: the panda://getting-started guide calls back into ListStatic/ListTemplates, which re-acquire the same read lock.

Go's sync.RWMutex refuses a recursive read lock once a writer is waiting. Datasource discovery registers resources for modules that activate after startup (local refresh every 5s), calling RegisterStatic/RegisterTemplate under the write lock. If such a register queues while a read is between its outer lock and the handler's inner lock, the reader blocks on the re-entrant read lock and the writer blocks on the reader, deadlocking. Because the read lock is then held forever, all later reads and the discovery refresh loop wedge until the process restarts.

Fix

Resolve the handler and mime type under the read lock, release the lock, then run the handler. A handler can now safely call back into the registry. Match order (static before template) and error messages are unchanged.

Tests

  • A read whose handler lists resources completes while a register runs concurrently. This test deadlocks on the old code and passes on the new code.
  • 32 readers running the getting-started handler race a stream of registers and all complete, verified with the race detector.

Full pkg/resource and pkg/server suites pass, including -race.

Read held the registry read lock while invoking a resource handler.
Handlers that list other resources, such as the getting-started guide,
re-acquire the read lock, which deadlocks against a concurrent register
from a datasource refresh once that register is queued on the write lock.

Resolve the handler under the lock and run it after releasing, so a
handler can safely call back into the registry. Add regression tests
covering a concurrent register during a resource-listing handler and
parallel reads racing a stream of registers.
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.

2 participants