diff --git a/Documentation/RelNotes/2.55.0.adoc b/Documentation/RelNotes/2.55.0.adoc index 328ea76714f993..63c921d9d0fc8e 100644 --- a/Documentation/RelNotes/2.55.0.adoc +++ b/Documentation/RelNotes/2.55.0.adoc @@ -148,6 +148,11 @@ Fixes since v2.54 commits deep, which has been corrected to be a no-op instead. (merge 2431f5e0e5 sp/shallow-deepen-on-non-shallow-repo-fix later to maint). + * "git maintenance" that goes background did not use the lockfile to + prevent multiple maintenance processes from running at the same + time, which has been corrected. + (merge 29364f1624 ps/maintenance-daemonize-lockfix later to maint). + * Other code cleanup, docfix, build fix, etc. (merge 80f4b802e9 ja/doc-difftool-synopsis-style later to maint). (merge b96490241e jc/doc-timestamps-in-stat later to maint). @@ -158,3 +163,5 @@ Fixes since v2.54 (merge ab9753e7bc kh/doc-restore-double-underscores-fix later to maint). (merge 4a9e097228 za/t2000-modernise-more later to maint). (merge b635fd0725 kh/doc-log-decorate-list later to maint). + (merge 65ea197dca jk/commit-sign-overflow-fix later to maint). + (merge 3ccb16052a jk/apply-leakfix later to maint). diff --git a/apply.c b/apply.c index 4aa1694cfaa2f0..249248d4f205ca 100644 --- a/apply.c +++ b/apply.c @@ -4881,8 +4881,10 @@ static int apply_patch(struct apply_state *state, state->patch_input_file = filename; state->linenr = 1; - if (read_patch_file(&buf, fd) < 0) - return -128; + if (read_patch_file(&buf, fd) < 0) { + res = -128; + goto end; + } offset = 0; while (offset < buf.len) { struct patch *patch; diff --git a/commit.c b/commit.c index f7ea9d99282503..e3e7352e69682d 100644 --- a/commit.c +++ b/commit.c @@ -1558,16 +1558,16 @@ int commit_tree(const char *msg, size_t msg_len, const struct object_id *tree, return result; } -static int find_invalid_utf8(const char *buf, int len) +static bool has_invalid_utf8(const char *buf, size_t len, size_t *bad_offset) { - int offset = 0; + size_t offset = 0; static const unsigned int max_codepoint[] = { 0x7f, 0x7ff, 0xffff, 0x10ffff }; while (len) { unsigned char c = *buf++; - int bytes, bad_offset; + unsigned bytes; unsigned int codepoint; unsigned int min_val, max_val; @@ -1578,7 +1578,7 @@ static int find_invalid_utf8(const char *buf, int len) if (c < 0x80) continue; - bad_offset = offset-1; + *bad_offset = offset-1; /* * Count how many more high bits set: that's how @@ -1595,11 +1595,11 @@ static int find_invalid_utf8(const char *buf, int len) * codepoints beyond U+10FFFF, which are guaranteed never to exist. */ if (bytes < 1 || 3 < bytes) - return bad_offset; + return true; /* Do we *have* that many bytes? */ if (len < bytes) - return bad_offset; + return true; /* * Place the encoded bits at the bottom of the value and compute the @@ -1617,23 +1617,23 @@ static int find_invalid_utf8(const char *buf, int len) codepoint <<= 6; codepoint |= *buf & 0x3f; if ((*buf++ & 0xc0) != 0x80) - return bad_offset; + return true; } while (--bytes); /* Reject codepoints that are out of range for the sequence length. */ if (codepoint < min_val || codepoint > max_val) - return bad_offset; + return true; /* Surrogates are only for UTF-16 and cannot be encoded in UTF-8. */ if ((codepoint & 0x1ff800) == 0xd800) - return bad_offset; + return true; /* U+xxFFFE and U+xxFFFF are guaranteed non-characters. */ if ((codepoint & 0xfffe) == 0xfffe) - return bad_offset; + return true; /* So are anything in the range U+FDD0..U+FDEF. */ if (codepoint >= 0xfdd0 && codepoint <= 0xfdef) - return bad_offset; + return true; } - return -1; + return false; } /* @@ -1645,15 +1645,14 @@ static int find_invalid_utf8(const char *buf, int len) static int ensure_utf8(struct strbuf *buf) { int ok = 1; - long pos = 0; + size_t pos = 0; for (;;) { - int bad; + size_t bad; unsigned char c; unsigned char replace[2]; - bad = find_invalid_utf8(buf->buf + pos, buf->len - pos); - if (bad < 0) + if (!has_invalid_utf8(buf->buf + pos, buf->len - pos, &bad)) return ok; pos += bad; ok = 0; diff --git a/run-command.c b/run-command.c index c146a56532a139..28202a81d83d8b 100644 --- a/run-command.c +++ b/run-command.c @@ -1944,10 +1944,14 @@ void run_processes_parallel(const struct run_process_parallel_opts *opts) int prepare_auto_maintenance(struct repository *r, int quiet, struct child_process *maint) { - int enabled, auto_detach; + int enabled = 1, auto_detach; - if (!repo_config_get_bool(r, "maintenance.auto", &enabled) && - !enabled) + if (repo_config_get_bool(r, "maintenance.auto", &enabled)) { + int gc_threshold; + if (!repo_config_get_int(r, "gc.auto", &gc_threshold)) + enabled = gc_threshold > 0; + } + if (!enabled) return 0; /* diff --git a/setup.c b/setup.c index 17c0662076487e..5580529e4923e3 100644 --- a/setup.c +++ b/setup.c @@ -2166,12 +2166,26 @@ int daemonize(void) errno = ENOSYS; return -1; #else - switch (fork()) { + pid_t parent_pid = getpid(); + pid_t child_pid = fork(); + + switch (child_pid) { case 0: + /* + * We're in the child process, so we take ownership of + * all tempfiles. + */ + reassign_tempfile_ownership(parent_pid, getpid()); break; case -1: die_errno(_("fork failed")); default: + /* + * We're in the parent process, so we drop ownership of + * all tempfiles to prevent us from removing them upon + * exit. + */ + reassign_tempfile_ownership(parent_pid, child_pid); exit(0); } if (setsid() == -1) diff --git a/setup.h b/setup.h index 80bc6e5f078af8..b5bc5f280c34f0 100644 --- a/setup.h +++ b/setup.h @@ -149,6 +149,21 @@ void verify_non_filename(const char *prefix, const char *name); int path_inside_repo(const char *prefix, const char *path); void sanitize_stdfds(void); + +/* + * Daemonize the current process by forking and then exiting the parent + * process. Returns 0 when successful, in which case the parent process will + * have exited and it's the child process that continues to run the code. + * Otherwise, a negative error code is returned and the parent process will + * continue execution. + * + * Note that this function will also perform the following changes: + * + * - Standard file descriptors in the child process are closed. + * - The child process is made a session leader via setsid(3p). + * - All tempfiles owned by the parent process are reassigned to the + * daemonized child process. + */ int daemonize(void); /* diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh index ed6f2bedb91919..d7f82e1bec163f 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -73,6 +73,31 @@ test_expect_success 'maintenance.auto config option' ' test_subcommand ! git maintenance run --auto --quiet --detach fifo-maint && + + { git maintenance run --task=prefetch --detach 7>&9 & } && + parent="$!" && + + # Reap the parent process so that the exec call below will not + # get SIGCHLD. + wait "$parent" && + + # Open the git-upload-pack(1) FIFO for writing, which will + # block until the upload-pack script opens it for reading. Once + # exec returns, we know that the daemonized child is alive and + # pinned. + exec 8>fifo-uploadpack && + + test_path_is_file .git/objects/maintenance.lock && + test_path_is_file .git/objects/"maintenance~pid.lock" && + + # Verify that the maintenance.lock still exists, and + # that it was created by the parent process, not the + # child. + echo "pid $parent" >expect && + test_cmp expect .git/objects/"maintenance~pid.lock" && + + # Reopen the maintenance FIFO as read-only so that + # git-maintenance(1) is the only writer. This will cause it to + # close the FIFO once the process exits. + exec 9<&- && + exec 9&- && + cat <&9 && + + test_path_is_missing .git/objects/maintenance.lock && + test_path_is_missing .git/objects/"maintenance~pid.lock" + ) +' + test_expect_success '--detach causes maintenance to run in background' ' test_when_finished "rm -rf repo" && git init repo && diff --git a/tempfile.c b/tempfile.c index 82dfa3d82f2ac9..f0fdf582794ba5 100644 --- a/tempfile.c +++ b/tempfile.c @@ -373,3 +373,15 @@ int delete_tempfile(struct tempfile **tempfile_p) return err ? -1 : 0; } + +void reassign_tempfile_ownership(pid_t from, pid_t to) +{ + volatile struct volatile_list_head *pos; + + list_for_each(pos, &tempfile_list) { + struct tempfile *p = list_entry(pos, struct tempfile, list); + + if (is_tempfile_active(p) && p->owner == from) + p->owner = to; + } +} diff --git a/tempfile.h b/tempfile.h index 2d2ae5b657d4a9..2227a095fd4289 100644 --- a/tempfile.h +++ b/tempfile.h @@ -282,4 +282,15 @@ int delete_tempfile(struct tempfile **tempfile_p); */ int rename_tempfile(struct tempfile **tempfile_p, const char *path); +/* + * Reassign ownership of all active tempfiles whose `owner` field matches + * `from` to `to`. + * + * This is intended for use by `daemonize()`; after `fork(2)`-ing, the parent + * transfers ownership to the daemonized child so that its atexit handler does + * not unlink tempfiles that should outlive it, and the child claims the + * inherited tempfiles so that they are cleaned up when the daemon exits. + */ +void reassign_tempfile_ownership(pid_t from, pid_t to); + #endif /* TEMPFILE_H */