Skip to content

Commit

Permalink
upload-pack: fix race condition in error messages
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
derrickstolee authored and dscho committed Aug 9, 2023
1 parent 98c7fc4 commit 29bf38f
Showing 1 changed file with 3 additions and 2 deletions.
5 changes: 3 additions & 2 deletions upload-pack.c
Original file line number Diff line number Diff line change
Expand Up @@ -801,11 +801,12 @@ static void check_non_tip(struct upload_pack_data *data)
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)) {
warning("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);
}
}
}
Expand Down

0 comments on commit 29bf38f

Please sign in to comment.