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: remove deadlock #785

Merged
merged 2 commits into from
Jun 21, 2022
Merged

libgit2: remove deadlock #785

merged 2 commits into from
Jun 21, 2022

Conversation

pjbgf
Copy link
Member

@pjbgf pjbgf commented Jun 16, 2022

Some scenarios may lead to deadlocks, specially in image automation controller.

Fix regression from #753 #775.

@pjbgf pjbgf added the area/git Git related issues and pull requests label Jun 16, 2022
Some scenarios may lead to deadlocks, specially
in image automation controller.

Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
@@ -117,7 +122,6 @@ type connection struct {
session *ssh.Session
currentStream *sshSmartSubtransportStream
connected bool
m sync.RWMutex
Copy link
Contributor

Choose a reason for hiding this comment

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

I like this approach, no more locking and safer.
This whole connection type was created in order to group these variables and lock them together. Since we no longer lock with this change, we can put them back into sshSmartSubtransport struct.

Copy link
Contributor

Choose a reason for hiding this comment

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

Pushed a commit with this change, as Paulo is away at the moment.

The connection type was created to group the connection related fields
and use mutex to prevent race conditions. Since that's no longer the
case, this puts back those fields in sshSmartSubtransport.

Signed-off-by: Sunny <darkowlzz@protonmail.com>
@darkowlzz darkowlzz requested a review from hiddeco June 21, 2022 12:47
@darkowlzz darkowlzz merged commit 2295134 into main Jun 21, 2022
@darkowlzz darkowlzz deleted the mutexless branch June 21, 2022 14:21
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
None yet
Development

Successfully merging this pull request may close these issues.

3 participants