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

libgit2/managed: fix race conditions in ssh transport #753

Merged
merged 1 commit into from
Jun 2, 2022

Conversation

darkowlzz
Copy link
Contributor

@darkowlzz darkowlzz commented Jun 2, 2022

Race conditions in ssh smart subtransport caused some goroutines to
panic, resulting in crashing the whole controller, mostly evident in
image-automation-controller CI runs. Panic recovery in the main thread
do not handle goroutine panics. So, the existing panic recovery code in
libgit2 Checkout() methods weren't able to handle it.

This change groups the fields in ssh smart subtransport that may be
accessed by multiple goroutines into a new struct with a mutex. Also
adds panic recovery in the created goroutine to handle any other
possible panics.

IAC CI failures due to the panic: https://github.com/fluxcd/image-automation-controller/runs/6711433986?check_suite_focus=true#step:5:578

Fix verified using IAC CI in fluxcd/image-automation-controller#376

Race conditions in ssh smart subtransport caused some goroutines to
panic, resulting in crashing the whole controller, mostly evident in
image-automation-controller CI runs. Panic recovery in the main thread
do not handle goroutine panics. So, the existing panic recovery code in
libgit2 Checkout() methods weren't able to handle it.

This change groups the fields in ssh smart subtransport that may be
accessed by multiple goroutines into a new struct with a mutex. Also
adds panic recovery in the created goroutine to handle any other
possible panics.

Signed-off-by: Sunny <darkowlzz@protonmail.com>
@darkowlzz darkowlzz added the area/git Git related issues and pull requests label Jun 2, 2022
@darkowlzz darkowlzz changed the title libgit2/managed: fix race issues in ssh transport libgit2/managed: fix race conditions in ssh transport Jun 2, 2022
Copy link
Member

@aryan9600 aryan9600 left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

@darkowlzz darkowlzz merged commit c5a5707 into main Jun 2, 2022
@darkowlzz darkowlzz deleted the libgit2-ssh-race-fixes branch June 2, 2022 20:46
@pjbgf pjbgf mentioned this pull request Jun 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/git Git related issues and pull requests
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants