Skip to content
This repository was archived by the owner on Apr 26, 2024. It is now read-only.

Fix rare deadlock when using read/write locks#16133

Merged
erikjohnston merged 2 commits intodevelopfrom
erikj/fix_deadlock
Aug 23, 2023
Merged

Fix rare deadlock when using read/write locks#16133
erikjohnston merged 2 commits intodevelopfrom
erikj/fix_deadlock

Conversation

@erikjohnston
Copy link
Copy Markdown
Member

Introduced in #15782

@erikjohnston erikjohnston marked this pull request as ready for review August 18, 2023 12:22
@erikjohnston erikjohnston requested a review from a team as a code owner August 18, 2023 12:22
@reivilibre
Copy link
Copy Markdown
Contributor

I can't really follow what change this is making or what race it's trying to cover up, any chance you can elaborate? :)

@erikjohnston
Copy link
Copy Markdown
Member Author

Whoops! Yes, basically: we have a _mode table that tracks (for a given lock name/key) that tracks whether its been locked and if so if its a read or a write lock and a token of a currently held lock. When we drop a lock we need to either delete that row, or update the token to another token (if there is another read lock).

If we try and drop two locks at the same time, we'll try and update the _mode table to point to the token of the other lock (that we're also dropping in another transaction), and that results in a deadlock.

The fix here is to make sure that we "serialise" changes to the lock with a given name/key. We do this by taking a row level lock on _mode table before we insert and delete (and we already do that for insert).

Copy link
Copy Markdown
Contributor

@reivilibre reivilibre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not dead obvious (but mostly just because I didn't have a clue how this locking stuff was put together) but makes sense now.

@erikjohnston erikjohnston merged commit 4adaba9 into develop Aug 23, 2023
@erikjohnston erikjohnston deleted the erikj/fix_deadlock branch August 23, 2023 12:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants