git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: gitster@pobox.com, Derrick Stolee <derrickstolee@github.com>,
	Derrick Stolee <dstolee@microsoft.com>
Subject: [PATCH] upload-pack: fix race condition in error messages
Date: Thu, 10 Aug 2023 14:40:50 +0000	[thread overview]
Message-ID: <pull.1572.git.1691678450757.gitgitgadget@gmail.com> (raw)

From: Derrick Stolee <dstolee@microsoft.com>

Test t5516-fetch-push.sh has a test 'deny fetch unreachable SHA1,
allowtipsha1inwant=true' that checks stderr for a specific error
string from the remote. In some build environments the error sent
over the remote connection gets mingled with the error from the
die() statement. Since both signals are being output to the same
file descriptor (but from parent and child processes), the output
we are matching with grep gets split.

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.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
    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
-- 
gitgitgadget

             reply	other threads:[~2023-08-10 14:40 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-10 14:40 Derrick Stolee via GitGitGadget [this message]
2023-08-10 16:14 ` [PATCH] upload-pack: fix race condition in error messages Junio C Hamano
2023-08-16  6:06 ` [PATCH] upload-pack: fix exit code when denying fetch of unreachable object ID Patrick Steinhardt
2023-08-16 16:16   ` Junio C Hamano
2023-08-16 16:44     ` Junio C Hamano
     [not found]       ` <CABQH79pick0c1UVc+W8n2QtVmSJAjqXcJGtYSm0aahAFDNvE1g@mail.gmail.com>
2023-08-17  5:12         ` Junio C Hamano
2023-08-17 10:07           ` Patrick Steinhardt
2023-08-17  5:27       ` Jeff King
2023-08-16 17:04     ` [Leftoverbits] exit code clean-up? Junio C Hamano
2023-08-17  5:36       ` Jeff King
2023-08-17 16:03         ` Junio C Hamano
2023-08-17  9:24       ` Oswald Buddenhagen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.1572.git.1691678450757.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=derrickstolee@github.com \
    --cc=dstolee@microsoft.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).