Skip to content

Commit 68f431b

Browse files
HaraldNordgrengitster
authored andcommitted
checkout: rollback lock on early returns in merge_working_tree
merge_working_tree() acquires the index lock via repo_hold_locked_index() but several early return paths exit without calling rollback_lock_file(), leaving the lock held. While this is currently harmless because the process exits soon after, it becomes a problem if the function is ever called more than once in the same process. Add rollback_lock_file() calls to all early return paths. Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent fc2afe3 commit 68f431b

File tree

1 file changed

+22
-7
lines changed

1 file changed

+22
-7
lines changed

builtin/checkout.c

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -783,8 +783,10 @@ static int merge_working_tree(const struct checkout_opts *opts,
783783
struct tree *new_tree;
784784

785785
repo_hold_locked_index(the_repository, &lock_file, LOCK_DIE_ON_ERROR);
786-
if (repo_read_index_preload(the_repository, NULL, 0) < 0)
786+
if (repo_read_index_preload(the_repository, NULL, 0) < 0) {
787+
rollback_lock_file(&lock_file);
787788
return error(_("index file corrupt"));
789+
}
788790

789791
resolve_undo_clear_index(the_repository->index);
790792
if (opts->new_orphan_branch && opts->orphan_from_empty_tree) {
@@ -797,14 +799,18 @@ static int merge_working_tree(const struct checkout_opts *opts,
797799
} else {
798800
new_tree = repo_get_commit_tree(the_repository,
799801
new_branch_info->commit);
800-
if (!new_tree)
802+
if (!new_tree) {
803+
rollback_lock_file(&lock_file);
801804
return error(_("unable to read tree (%s)"),
802805
oid_to_hex(&new_branch_info->commit->object.oid));
806+
}
803807
}
804808
if (opts->discard_changes) {
805809
ret = reset_tree(new_tree, opts, 1, writeout_error, new_branch_info);
806-
if (ret)
810+
if (ret) {
811+
rollback_lock_file(&lock_file);
807812
return ret;
813+
}
808814
} else {
809815
struct tree_desc trees[2];
810816
struct tree *tree;
@@ -814,6 +820,7 @@ static int merge_working_tree(const struct checkout_opts *opts,
814820
refresh_index(the_repository->index, REFRESH_QUIET, NULL, NULL, NULL);
815821

816822
if (unmerged_index(the_repository->index)) {
823+
rollback_lock_file(&lock_file);
817824
error(_("you need to resolve your current index first"));
818825
return 1;
819826
}
@@ -857,15 +864,19 @@ static int merge_working_tree(const struct checkout_opts *opts,
857864
struct strbuf sb = STRBUF_INIT;
858865
struct strbuf old_commit_shortname = STRBUF_INIT;
859866

860-
if (!opts->merge)
867+
if (!opts->merge) {
868+
rollback_lock_file(&lock_file);
861869
return 1;
870+
}
862871

863872
/*
864873
* Without old_branch_info->commit, the below is the same as
865874
* the two-tree unpack we already tried and failed.
866875
*/
867-
if (!old_branch_info->commit)
876+
if (!old_branch_info->commit) {
877+
rollback_lock_file(&lock_file);
868878
return 1;
879+
}
869880
old_tree = repo_get_commit_tree(the_repository,
870881
old_branch_info->commit);
871882

@@ -897,8 +908,10 @@ static int merge_working_tree(const struct checkout_opts *opts,
897908
ret = reset_tree(new_tree,
898909
opts, 1,
899910
writeout_error, new_branch_info);
900-
if (ret)
911+
if (ret) {
912+
rollback_lock_file(&lock_file);
901913
return ret;
914+
}
902915
o.ancestor = old_branch_info->name;
903916
if (!old_branch_info->name) {
904917
strbuf_add_unique_abbrev(&old_commit_shortname,
@@ -920,8 +933,10 @@ static int merge_working_tree(const struct checkout_opts *opts,
920933
writeout_error, new_branch_info);
921934
strbuf_release(&o.obuf);
922935
strbuf_release(&old_commit_shortname);
923-
if (ret)
936+
if (ret) {
937+
rollback_lock_file(&lock_file);
924938
return ret;
939+
}
925940
}
926941
}
927942

0 commit comments

Comments
 (0)