Re: [PATCH] upload-pack: fix race condition in error messages
From: Junio C Hamano <hidden>
Date: 2023-08-10 16:14:28
"Derrick Stolee via GitGitGadget" [off-list ref] writes:
To reduce the risk of this failure, follow this process instead: 1. Write an error message to stderr. 2. Write an error message across the connection. 3. exit(1). This reorders the events so the error is written entirely before the client receives a message from the remote, removing the race condition.
Makes sense. error() eventually goes to our own vreportf() that flushes, so by the time the call returns, our message should be already out, and the other message has no chance to be mixed with it. Will queue. Thanks.
quoted hunk
Signed-off-by: Derrick Stolee <redacted> --- upload-pack: fix race condition in error messages Here is another quick patch that we've been holding in the microsoft/git fork for years because it helped prevent some test flakiness, especially in our more involved functional test environment. Thanks, -Stolee Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1572%2Fderrickstolee%2Fupload-pack-race-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1572/derrickstolee/upload-pack-race-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/1572 upload-pack.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)diff --git a/upload-pack.c b/upload-pack.c index 94751477ab2..7b25129f0f6 100644 --- a/upload-pack.c +++ b/upload-pack.c@@ -801,11 +801,12 @@ error: for (i = 0; i < data->want_obj.nr; i++) { struct object *o = data->want_obj.objects[i].item; if (!is_our_ref(o, data->allow_uor)) { + error("git upload-pack: not our ref %s", + oid_to_hex(&o->oid)); packet_writer_error(&data->writer, "upload-pack: not our ref %s", oid_to_hex(&o->oid)); - die("git upload-pack: not our ref %s", - oid_to_hex(&o->oid)); + exit(1); } } }base-commit: a82fb66fed250e16d3010c75404503bea3f0ab61