Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIXED] JetStream: possible deadlock due to lock inversion #2479

Merged
merged 3 commits into from
Aug 31, 2021

Conversation

kozlovic
Copy link
Member

The locking is jetStream->Server, not the otherway around. There
was few places where lock inversion could have caused deadlock.

Also, a change made recently to solve a deadlock was causing
a race that is demonstrated with TestJetStreamRaceOnRAFTCreate.

Signed-off-by: Ivan Kozlovic ivan@synadia.com

The locking is jetStream->Server, not the otherway around. There
was few places where lock inversion could have caused deadlock.

Also, a change made recently to solve a deadlock was causing
a race that is demonstrated with TestJetStreamRaceOnRAFTCreate.

Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
@kozlovic kozlovic requested a review from derekcollison August 30, 2021 22:50
Order seem to be jetStream -> jsAccount. In JetStreamUsage()
we were doing the opposite. Moved the js.mu.RLock() to encompass
the whole getting of statistics from jsa. We could do in 2 phases:
get js's RLock to get cluster info (if clustered), then get
jsa's RLock for the rest, and set cluster info with what we gathered
in the first phase.

Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
Copy link
Member

@derekcollison derekcollison left a comment

Choose a reason for hiding this comment

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

LGTM

server/monitor.go Show resolved Hide resolved
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
@derekcollison derekcollison self-requested a review August 31, 2021 21:22
Copy link
Member

@derekcollison derekcollison left a comment

Choose a reason for hiding this comment

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

LGTM

@kozlovic kozlovic merged commit 2539bbb into main Aug 31, 2021
@kozlovic kozlovic deleted the fix_js_deadlock_and_race branch August 31, 2021 21:27
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