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

Fix deadlock in token reloading #830

Merged
merged 2 commits into from
Feb 16, 2022
Merged

Fix deadlock in token reloading #830

merged 2 commits into from
Feb 16, 2022

Conversation

clux
Copy link
Member

@clux clux commented Feb 16, 2022

lock at one point before entering the branch to prevent two locks being attempted

@clux clux linked an issue Feb 16, 2022 that may be closed by this pull request
@clux clux added the changelog-fix changelog fix category for prs label Feb 16, 2022
@clux clux requested a review from a team February 16, 2022 17:26
@clux
Copy link
Member Author

clux commented Feb 16, 2022

ideally, we could release 0.69.1 and yank 0.69.0 if this is viable.

one slight complication: #827 was merged before with a breaking change

so i propose we revert that temporarily, release 0.69.1, then merge it back. wdyt @teozkr ?

Copy link
Member

@nightkr nightkr left a comment

Choose a reason for hiding this comment

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

The solution makes sense to me, but I can't seem to repro the original issue...

@nightkr
Copy link
Member

nightkr commented Feb 16, 2022

@clux Or we could create a separate 0.69 branch that we cherry-pick this merge into.

@clux
Copy link
Member Author

clux commented Feb 16, 2022

yeah, that would be even better actually.

@nightkr
Copy link
Member

nightkr commented Feb 16, 2022

Ok, got a repro running, and this seems to fix it.

@nightkr nightkr merged commit d8bc6e4 into master Feb 16, 2022
@nightkr nightkr deleted the test-hangs2 branch February 16, 2022 17:41
nightkr added a commit to nightkr/kube-rs that referenced this pull request Feb 16, 2022
nightkr added a commit to nightkr/kube-rs that referenced this pull request Feb 16, 2022
Fix deadlock in token reloading

Signed-off-by: Teo Klestrup Röijezon <[email protected]>
bearer_header(token)
} else {
bearer_header(token_file.write().await.token())
bearer_header(locked.token())
}
Copy link
Member

Choose a reason for hiding this comment

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

So, this can lead to the deadlock explained in the docs?

Note that under the priority policy of RwLock, read locks are not granted until prior write locks, to prevent starvation. Therefore deadlock may occur if a read lock is held by the current task, a write lock attempt is made, and then a subsequent read lock attempt is made by the current task.

Returns an RAII guard which will drop this read access of the RwLock when dropped.

I thought the RwLockReadGuard from .read().await is dropped before the else block :/

Copy link
Member

Choose a reason for hiding this comment

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

So did I, to be honest.. :/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog-fix changelog fix category for prs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Weird intermittent hangs in tower after updating to 0.69
3 participants