diff --git a/refs.c b/refs.c index 58b118243c..85c1dcb017 100644 --- a/refs.c +++ b/refs.c @@ -30,6 +30,13 @@ static unsigned char refname_disposition[256] = { * pruned. */ #define REF_ISPRUNING 0x0100 + +/* + * Used as a flag in ref_update::flags when the lockfile needs to be + * committed. + */ +#define REF_NEEDS_COMMIT 0x0200 + /* * Try to read one refname component from the front of refname. * Return the length of the component found, or -1 if the component is @@ -3799,7 +3806,12 @@ int ref_transaction_commit(struct ref_transaction *transaction, goto cleanup; } - /* Acquire all locks while verifying old values */ + /* + * Acquire all locks, verify old values if provided, check + * that new values are valid, and write new values to the + * lockfiles, ready to be activated. Only keep one lockfile + * open at a time to avoid running out of file descriptors. + */ for (i = 0; i < n; i++) { struct ref_update *update = updates[i]; @@ -3820,26 +3832,49 @@ int ref_transaction_commit(struct ref_transaction *transaction, update->refname); goto cleanup; } + if (!(update->flags & REF_DELETING) && + (update->lock->force_write || + hashcmp(update->lock->old_sha1, update->new_sha1))) { + if (write_ref_to_lockfile(update->lock, update->new_sha1)) { + /* + * The lock was freed upon failure of + * write_ref_to_lockfile(): + */ + update->lock = NULL; + strbuf_addf(err, "Cannot update the ref '%s'.", + update->refname); + ret = TRANSACTION_GENERIC_ERROR; + goto cleanup; + } + update->flags |= REF_NEEDS_COMMIT; + } else { + /* + * We didn't have to write anything to the lockfile. + * Close it to free up the file descriptor: + */ + if (close_ref(update->lock)) { + strbuf_addf(err, "Couldn't close %s.lock", + update->refname); + goto cleanup; + } + } } /* Perform updates first so live commits remain referenced */ for (i = 0; i < n; i++) { struct ref_update *update = updates[i]; - if (!(update->flags & REF_DELETING)) { - if (!update->lock->force_write && - !hashcmp(update->lock->old_sha1, update->new_sha1)) { - unlock_ref(update->lock); + if (update->flags & REF_NEEDS_COMMIT) { + if (commit_ref_update(update->lock, + update->new_sha1, update->msg)) { + /* The lock was freed by commit_ref_update(): */ update->lock = NULL; - } else if (write_ref_to_lockfile(update->lock, update->new_sha1) || - commit_ref_update(update->lock, update->new_sha1, update->msg)) { - update->lock = NULL; /* freed by the above calls */ strbuf_addf(err, "Cannot update the ref '%s'.", update->refname); ret = TRANSACTION_GENERIC_ERROR; goto cleanup; } else { - /* freed by the above calls: */ + /* freed by the above call: */ update->lock = NULL; } } @@ -3849,7 +3884,7 @@ int ref_transaction_commit(struct ref_transaction *transaction, for (i = 0; i < n; i++) { struct ref_update *update = updates[i]; - if (update->lock) { + if (update->flags & REF_DELETING) { if (delete_ref_loose(update->lock, update->type, err)) { ret = TRANSACTION_GENERIC_ERROR; goto cleanup; diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh index 47d2fe9cce..c593a1df20 100755 --- a/t/t1400-update-ref.sh +++ b/t/t1400-update-ref.sh @@ -979,7 +979,7 @@ run_with_limited_open_files () { test_lazy_prereq ULIMIT_FILE_DESCRIPTORS 'run_with_limited_open_files true' -test_expect_failure ULIMIT_FILE_DESCRIPTORS 'large transaction creating branches does not burst open file limit' ' +test_expect_success ULIMIT_FILE_DESCRIPTORS 'large transaction creating branches does not burst open file limit' ' ( for i in $(test_seq 33) do @@ -990,7 +990,7 @@ test_expect_failure ULIMIT_FILE_DESCRIPTORS 'large transaction creating branches ) ' -test_expect_failure ULIMIT_FILE_DESCRIPTORS 'large transaction deleting branches does not burst open file limit' ' +test_expect_success ULIMIT_FILE_DESCRIPTORS 'large transaction deleting branches does not burst open file limit' ' ( for i in $(test_seq 33) do