helping smart-http/stateless-rpc fetch race
A request to fetch from a client over smart HTTP protocol is served in multiple steps. In the first round, the server side shows the set of refs it has and their values, and the client picks from them and sends "I want to fetch the history leading to these commits". When the server tries to respond to this second request, its refs may have progressed by a push from elsewhere. By design, we do not allow fetching objects that are not at the tip of an advertised ref, and the server rejects such a request. The client needs to try again, which is not ideal especially for a busy server. Teach upload-pack (which is the workhorse driven by git-daemon and smart http server interface) that it is OK for a smart-http client to ask for commits that are not at the tip of any advertised ref, as long as they are reachable from advertised refs. Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano committed
Aug 5, 2011 at 13:54 UTC
051e4005a37c4ebe8b8855e415e6c6969d68e1a3
1 file changed
+98
-9
upload-pack.c
+98
-9
@@ -10,6 +10,7 @@
10
#include "revision.h"
11
#include "list-objects.h"
12
#include "run-command.h"
13
+#include "sigchain.h"
14
15
static const char upload_pack_usage[] = "git upload-pack [--strict] [--timeout=<n>] <dir>";
16
@@ -498,11 +499,95 @@ static int get_common_commits(void)
499
}
500
}
501
502
+static void check_non_tip(void)
503
+{
504
+ static const char *argv[] = {
505
+ "rev-list", "--stdin", NULL,
506
+ };
507
+ static struct child_process cmd;
508
+ struct object *o;
509
+ char namebuf[42]; /* ^ + SHA-1 + LF */
510
+ int i;
511
+
512
+ /* In the normal in-process case non-tip request can never happen */
513
+ if (!stateless_rpc)
514
+ goto error;
515
+
516
+ cmd.argv = argv;
517
+ cmd.git_cmd = 1;
518
+ cmd.no_stderr = 1;
519
+ cmd.in = -1;
520
+ cmd.out = -1;
521
+
522
+ if (start_command(&cmd))
523
+ goto error;
524
+
525
+ /*
526
+ * If rev-list --stdin encounters an unknown commit, it
527
+ * terminates, which will cause SIGPIPE in the write loop
528
+ * below.
529
+ */
530
+ sigchain_push(SIGPIPE, SIG_IGN);
531
+
532
+ namebuf[0] = '^';
533
+ namebuf[41] = '\n';
534
+ for (i = get_max_object_index(); 0 < i; ) {
535
+ o = get_indexed_object(--i);
536
+ if (!(o->flags & OUR_REF))
537
+ continue;
538
+ memcpy(namebuf + 1, sha1_to_hex(o->sha1), 40);
539
+ if (write_in_full(cmd.in, namebuf, 42) < 0)
540
+ goto error;
541
+ }
542
+ namebuf[40] = '\n';
543
+ for (i = 0; i < want_obj.nr; i++) {
544
+ o = want_obj.objects[i].item;
545
+ if (o->flags & OUR_REF)
546
+ continue;
547
+ memcpy(namebuf, sha1_to_hex(o->sha1), 40);
548
+ if (write_in_full(cmd.in, namebuf, 41) < 0)
549
+ goto error;
550
+ }
551
+ close(cmd.in);
552
+
553
+ sigchain_pop(SIGPIPE);
554
+
555
+ /*
556
+ * The commits out of the rev-list are not ancestors of
557
+ * our ref.
558
+ */
559
+ i = read_in_full(cmd.out, namebuf, 1);
560
+ if (i)
561
+ goto error;
562
+ close(cmd.out);
563
+
564
+ /*
565
+ * rev-list may have died by encountering a bad commit
566
+ * in the history, in which case we do want to bail out
567
+ * even when it showed no commit.
568
+ */
569
+ if (finish_command(&cmd))
570
+ goto error;
571
+
572
+ /* All the non-tip ones are ancestors of what we advertised */
573
+ return;
574
+
575
+error:
576
+ /* Pick one of them (we know there at least is one) */
577
+ for (i = 0; i < want_obj.nr; i++) {
578
+ o = want_obj.objects[i].item;
579
+ if (!(o->flags & OUR_REF))
580
+ die("git upload-pack: not our ref %s",
581
+ sha1_to_hex(o->sha1));
582
+ }
583
+}
584
+
585
static void receive_needs(void)
586
{
587
struct object_array shallows = OBJECT_ARRAY_INIT;
588
static char line[1000];
589
int len, depth = 0;
590
+ int has_non_tip = 0;
591
592
shallow_nr = 0;
593
if (debug_fd)
@@ -559,26 +644,30 @@ static void receive_needs(void)
644
if (strstr(line+45, "include-tag"))
645
use_include_tag = 1;
646
562
- /* We have sent all our refs already, and the other end
563
- * should have chosen out of them; otherwise they are
564
- * asking for nonsense.
565
- *
566
- * Hmph. We may later want to allow "want" line that
567
- * asks for something like "master~10" (symbolic)...
568
- * would it make sense? I don't know.
569
- */
647
o = lookup_object(sha1_buf);
571
- if (!o || !(o->flags & OUR_REF))
648
+ if (!o)
649
die("git upload-pack: not our ref %s",
650
sha1_to_hex(sha1_buf));
651
if (!(o->flags & WANTED)) {
652
o->flags |= WANTED;
653
+ if (!(o->flags & OUR_REF))
654
+ has_non_tip = 1;
655
add_object_array(o, NULL, &want_obj);
656
}
657
}
658
if (debug_fd)
659
write_str_in_full(debug_fd, "#E\n");
660
661
+ /*
662
+ * We have sent all our refs already, and the other end
663
+ * should have chosen out of them. When we are operating
664
+ * in the stateless RPC mode, however, their choice may
665
+ * have been based on the set of older refs advertised
666
+ * by another process that handled the initial request.
667
+ */
668
+ if (has_non_tip)
669
+ check_non_tip();
670
+
671
if (!use_sideband && daemon_mode)
672
no_progress = 1;
673