upload-pack: fix race condition in error messages

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> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Derrick Stolee committed Aug 10, 2023 at 14:40 UTC 7ba7c52d76630183b47c57bdd2da8eea033bdd2f
1 file changed +3 -2
upload-pack.c
+3 -2
@@ -777,11 +777,12 @@ error:
777 for (i = 0; i < data->want_obj.nr; i++) {
778 struct object *o = data->want_obj.objects[i].item;
779 if (!is_our_ref(o, data->allow_uor)) {
780 + error("git upload-pack: not our ref %s",
781 + oid_to_hex(&o->oid));
782 packet_writer_error(&data->writer,
783 "upload-pack: not our ref %s",
784 oid_to_hex(&o->oid));
783 - die("git upload-pack: not our ref %s",
784 - oid_to_hex(&o->oid));
785 + exit(1);
786 }
787 }
788 }