Skip to content

refs: unlock unmodified refs on transaction commit#5264

Merged
pks-t merged 1 commit intolibgit2:masterfrom
henkesn:refs-unlock-on-commit
Oct 24, 2019
Merged

refs: unlock unmodified refs on transaction commit#5264
pks-t merged 1 commit intolibgit2:masterfrom
henkesn:refs-unlock-on-commit

Conversation

@henkesn
Copy link
Contributor

@henkesn henkesn commented Oct 11, 2019

Refs which are locked in a transaction without an altered target,
still should to be unlocked on git_transaction_commit.
git_transaction_free also unlocks refs but the moment of calling of git_transaction_free
cannot be controlled in all situations.
Some binding libs call git_transaction_free on garbage collection or not at all if the
application exits before and don't provide public access to git_transaction_free.
It is better to release locks as soon as possible.

@henkesn
Copy link
Contributor Author

henkesn commented Oct 15, 2019

Btw, would be good to have this released together with #5257 because the working locks could lead to running into this issue more frequently.

Copy link
Member

@pks-t pks-t left a comment

Choose a reason for hiding this comment

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

Thanks for this pull request! Looks good to me, except for the missing error check

Refs which are locked in a transaction without an altered target,
still should to be unlocked on `git_transaction_commit`.
`git_transaction_free` also unlocks refs but the moment of calling of `git_transaction_free`
cannot be controlled in all situations.
Some binding libs call `git_transaction_free` on garbage collection or not at all if the
application exits before and don't provide public access to `git_transaction_free`.
It is better to release locks as soon as possible.
@henkesn henkesn requested a review from pks-t October 18, 2019 09:22
@pks-t pks-t merged commit c405f23 into libgit2:master Oct 24, 2019
@pks-t
Copy link
Member

pks-t commented Oct 24, 2019

Thanks again for your fix!

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