Skip to content

refs: unlock unmodified refs on transaction commit#5264

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

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

Conversation

@henkesn

@henkesn henkesn commented Oct 11, 2019

Copy link
Copy Markdown
Contributor

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 commented Oct 15, 2019

Copy link
Copy Markdown
Contributor Author

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

@pks-t pks-t left a comment

Copy link
Copy Markdown
Member

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

Comment thread src/transaction.c Outdated
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

pks-t commented Oct 24, 2019

Copy link
Copy Markdown
Member

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