{"thread":{"id":"39614","subject":"git push keeps writing after server failure","startedAt":"2015-06-12T17:31:33Z","lastAt":"2015-06-12T18:50:16Z","messageCount":5,"participants":["Shawn Pearce","Junio C Hamano","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"263677","messageId":"CAJo=hJvzjnLFhF4REBzX=pgFamBfu7hZeZidy_8Rmi_DwwR4Nw@mail.gmail.com","threadId":"39614","inReplyTo":null,"subject":"git push keeps writing after server failure","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2015-06-12T17:31:33Z","receivedAt":"2015-06-12T17:31:33Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"I did something stupid like trying to push a copy of WebKit[1] into my\nGitHub account. This is ~5.2 GiB of data, which GitHub prefers not to\naccept. Ok ...\n\n$ git push --all git@github.com:spearce/wk.git\nCounting objects: 2752427, done.\nDelta compression using up to 12 threads.\nCompressing objects: 100% (442684/442684), done.\nremote: fatal: pack exceeds maximum allowed size\nWriting objects: 100% (2752427/2752427), 5.28 GiB | 8.86 MiB/s, done.\nTotal 2752427 (delta 2225007), reused 2752427 (delta 2225007)\nfatal: The remote end hung up unexpectedly\nfatal: The remote end hung up unexpectedly\n\nNotice GitHub prints \"remote: fatal: pack exceeds maximum allowed\nsize\". That interrupted my \"Writing objects\" progress meter, and then\ngit push just kept going and wrote really really fast (170 MiB/s!)\nuntil the entire pack was sent.\n\nA similar thing happens on https:// if the remote HTTP server goes\naway in the middle of sending the pack. Except its slower to send the\nremainder of the pack before git push finally terminates with an\nerror.\n\nShouldn't git push realize its stream is broken and stop writing when\nthe peer is all like \"uh, no, I'm not going to do that, but thanks for\ntrying\"?\n\n\n[1] https://webkit.googlesource.com/WebKit/\n"},{"id":"263681","messageId":"xmqqtwucdd31.fsf@gitster.dls.corp.google.com","threadId":"39614","inReplyTo":"CAJo=hJvzjnLFhF4REBzX=pgFamBfu7hZeZidy_8Rmi_DwwR4Nw@mail.gmail.com","subject":"Re: git push keeps writing after server failure","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-12T17:57:22Z","receivedAt":"2015-06-12T17:57:22Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shawn Pearce <spearce@spearce.org> writes:\n\n> I did something stupid like trying to push a copy of WebKit[1] into my\n> GitHub account. This is ~5.2 GiB of data, which GitHub prefers not to\n> accept. Ok ...\n> ...\n> Shouldn't git push realize its stream is broken and stop writing when\n> the peer is all like \"uh, no, I'm not going to do that, but thanks for\n> trying\"?\n\nNice wish, but I am not sure if we are structured to do that easily.\nThe \"fatal:\" message is going on the sideband that is demultiplexed\nby a separate async \"sideband-demux\" thread and pack_objects() is\noblivious to what is being said on the error channel #2.\n"},{"id":"263684","messageId":"20150612181256.GB9242@peff.net","threadId":"39614","inReplyTo":"CAJo=hJvzjnLFhF4REBzX=pgFamBfu7hZeZidy_8Rmi_DwwR4Nw@mail.gmail.com","subject":"Re: git push keeps writing after server failure","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-06-12T18:12:56Z","receivedAt":"2015-06-12T18:12:56Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jun 12, 2015 at 10:31:33AM -0700, Shawn Pearce wrote:\n\n> I did something stupid like trying to push a copy of WebKit[1] into my\n> GitHub account. This is ~5.2 GiB of data, which GitHub prefers not to\n> accept. Ok ...\n\nHeh, yeah. We cap it at 2G, and if you are going to have a WebKit fork,\nwe prefer you fork the actual WebKit repo so it shares objects (though\nif you have a need to create a new fork network, let me know).\n\n> $ git push --all git@github.com:spearce/wk.git\n> Counting objects: 2752427, done.\n> Delta compression using up to 12 threads.\n> Compressing objects: 100% (442684/442684), done.\n> remote: fatal: pack exceeds maximum allowed size\n> Writing objects: 100% (2752427/2752427), 5.28 GiB | 8.86 MiB/s, done.\n> Total 2752427 (delta 2225007), reused 2752427 (delta 2225007)\n> fatal: The remote end hung up unexpectedly\n> fatal: The remote end hung up unexpectedly\n> \n> Notice GitHub prints \"remote: fatal: pack exceeds maximum allowed\n> size\". That interrupted my \"Writing objects\" progress meter, and then\n> git push just kept going and wrote really really fast (170 MiB/s!)\n> until the entire pack was sent.\n\nSounds like it's writing to a closed fd, then. Which makes sense; I\nthink we should hang up the socket after writing the \"fatal\" message\nabove.\n\n> Shouldn't git push realize its stream is broken and stop writing when\n> the peer is all like \"uh, no, I'm not going to do that, but thanks for\n> trying\"?\n\nHrm. I have this old patch, which was originally written so that \"kill\n$(pidof git-push)\" did not let a rogue pack-objects continue writing.\n\nI'm not sure if that's what is going on here, though. I think we connect\npack-objects directly to the socket. So it sounds more like\n\"pack-objects --stdout\" needs to know to stop writing when writes to the\nsocket fail.\n\n-- >8 --\nDate: Sun, 3 Apr 2011 20:53:08 -0400\nSubject: [PATCH] send-pack: kill pack-objects helper on signal or exit\n\nWe spawn an external pack-objects process to actually send\nobjects to the remote side. If we are killed by a signal\nduring this process, the pack-objects will keep running and\ncomplete the push, which may surprise the user. We should\ntake it down when we go down.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n send-pack.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/send-pack.c b/send-pack.c\nindex 2a64fec..bdf723b 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -67,6 +67,7 @@ static int pack_objects(int fd, struct ref *refs, struct sha1_array *extra, stru\n \tpo.in = -1;\n \tpo.out = args->stateless_rpc ? -1 : fd;\n \tpo.git_cmd = 1;\n+\tpo.clean_on_exit = 1;\n \tif (start_command(&po))\n \t\tdie_errno(\"git pack-objects failed\");\n \n-- \n2.4.2.752.geeb594a\n"},{"id":"263686","messageId":"20150612182045.GA23698@peff.net","threadId":"39614","inReplyTo":"20150612181256.GB9242@peff.net","subject":"Re: git push keeps writing after server failure","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-06-12T18:20:45Z","receivedAt":"2015-06-12T18:20:45Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jun 12, 2015 at 02:12:56PM -0400, Jeff King wrote:\n\n> > $ git push --all git@github.com:spearce/wk.git\n> > Counting objects: 2752427, done.\n> > Delta compression using up to 12 threads.\n> > Compressing objects: 100% (442684/442684), done.\n> > remote: fatal: pack exceeds maximum allowed size\n> > Writing objects: 100% (2752427/2752427), 5.28 GiB | 8.86 MiB/s, done.\n> > Total 2752427 (delta 2225007), reused 2752427 (delta 2225007)\n> > fatal: The remote end hung up unexpectedly\n> > fatal: The remote end hung up unexpectedly\n> > \n> > Notice GitHub prints \"remote: fatal: pack exceeds maximum allowed\n> > size\". That interrupted my \"Writing objects\" progress meter, and then\n> > git push just kept going and wrote really really fast (170 MiB/s!)\n> > until the entire pack was sent.\n> \n> Sounds like it's writing to a closed fd, then. Which makes sense; I\n> think we should hang up the socket after writing the \"fatal\" message\n> above.\n\nFor reference, here's the patch implementing the max-size check on the\nserver. It's on my long list of patches to clean up and send to the\nlist; I never did this one because of the unpack-objects caveat\nmentioned below.\n\nThe death happens in index-pack. I think receive-pack should notice that\nand bail (closing the socket), but it's possible that it doesn't.\n\nThere's also a git-aware proxying layer at GitHub between clients and\ngit-receive-pack these days. It's possible that it is not properly\nhanging up until it sees EOF from the client.\n\nIf any of those things are true, then it is a GitHub-specific problem\n(which I still want to fix, but it is not something git upstream needs\nto care about).\n\n-- >8 --\nDate: Tue, 4 Sep 2012 12:01:06 -0400\nSubject: [PATCH] receive-pack: allow a maximum input size to be specified\n\nReceive-pack feeds its input to index-pack, which will\nhappily accept as many bytes as a sender is willing to\nprovide. Let's allow an arbitrary cutoff point where we will\nstop writing bytes to disk (they're still left in the\ntmp_pack, but cleaning that can easily be done outside of\nreceive-pack).\n\nNote that this doesn't handle the git-unpack-objects code\npath at all (because GitHub sets transfer.unpacklimit to 1,\nso we never unpack pushed objects anyway).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/index-pack.c   |  5 +++++\n builtin/receive-pack.c | 14 +++++++++++++-\n t/t5528-push-limits.sh | 27 +++++++++++++++++++++++++++\n 3 files changed, 45 insertions(+), 1 deletion(-)\n create mode 100755 t/t5528-push-limits.sh\n\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex dd1c5c9..054a144 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -70,6 +70,7 @@ static struct progress *progress;\n static unsigned char input_buffer[4096];\n static unsigned int input_offset, input_len;\n static off_t consumed_bytes;\n+static off_t max_input_size;\n static unsigned deepest_delta;\n static git_SHA_CTX input_ctx;\n static uint32_t input_crc32;\n@@ -167,6 +168,8 @@ static void use(int bytes)\n \tif (signed_add_overflows(consumed_bytes, bytes))\n \t\tdie(\"pack too large for current definition of off_t\");\n \tconsumed_bytes += bytes;\n+\tif (max_input_size && consumed_bytes > max_input_size)\n+\t\tdie(\"pack exceeds maximum allowed size\");\n }\n \n static const char *open_pack_file(const char *pack_name)\n@@ -1148,6 +1151,8 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)\n \t\t\t\t\topts.off32_limit = strtoul(c+1, &c, 0);\n \t\t\t\tif (*c || opts.off32_limit & 0x80000000)\n \t\t\t\t\tdie(\"bad %s\", arg);\n+\t\t\t} else if (!prefixcmp(arg, \"--max-input-size=\")) {\n+\t\t\t\tmax_input_size = strtoul(arg + 17, NULL, 10);\n \t\t\t} else\n \t\t\t\tusage(index_pack_usage);\n \t\t\tcontinue;\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 0afb8b2..e64b8d2 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -31,6 +31,7 @@ static int transfer_fsck_objects = -1;\n static int receive_unpack_limit = -1;\n static int transfer_unpack_limit = -1;\n static int unpack_limit = 100;\n+static off_t max_input_size;\n static int report_status;\n static int use_sideband;\n static int quiet;\n@@ -113,6 +114,11 @@ static int receive_pack_config(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \n+\tif (strcmp(var, \"receive.maxsize\") == 0) {\n+\t\tmax_input_size = git_config_ulong(var, value);\n+\t\treturn 0;\n+\t}\n+\n \treturn git_default_config(var, value, cb);\n }\n \n@@ -832,9 +838,10 @@ static const char *unpack(void)\n \t\t\treturn NULL;\n \t\treturn \"unpack-objects abnormal exit\";\n \t} else {\n-\t\tconst char *keeper[7];\n+\t\tconst char *keeper[8];\n \t\tint s, status, i = 0;\n \t\tchar keep_arg[256];\n+\t\tchar max_input_arg[256];\n \t\tstruct child_process ip;\n \n \t\ts = sprintf(keep_arg, \"--keep=receive-pack %\"PRIuMAX\" on \", (uintmax_t) getpid());\n@@ -848,6 +855,11 @@ static const char *unpack(void)\n \t\tkeeper[i++] = \"--fix-thin\";\n \t\tkeeper[i++] = hdr_arg;\n \t\tkeeper[i++] = keep_arg;\n+\t\tif (max_input_size) {\n+\t\t\tsnprintf(max_input_arg, sizeof(max_input_arg),\n+\t\t\t\t \"--max-input-size=%lu\", max_input_size);\n+\t\t\tkeeper[i++] = max_input_arg;\n+\t\t}\n \t\tkeeper[i++] = NULL;\n \t\tmemset(&ip, 0, sizeof(ip));\n \t\tip.argv = keeper;\ndiff --git a/t/t5528-push-limits.sh b/t/t5528-push-limits.sh\nnew file mode 100755\nindex 0000000..c2f6d77\n--- /dev/null\n+++ b/t/t5528-push-limits.sh\n@@ -0,0 +1,27 @@\n+#!/bin/sh\n+\n+test_description='check input limits for pushing'\n+. ./test-lib.sh\n+\n+test_expect_success 'create known-size commit' '\n+\ttest-genrandom foo 1024 >file &&\n+\tgit add file &&\n+\ttest_commit one-k\n+'\n+\n+test_expect_success 'create remote repository' '\n+\tgit init --bare dest &&\n+\tgit --git-dir=dest config receive.unpacklimit 1\n+'\n+\n+test_expect_success 'receive.maxsize rejects push' '\n+\tgit --git-dir=dest config receive.maxsize 512 &&\n+\ttest_must_fail git push dest HEAD\n+'\n+\n+test_expect_success 'bumping limit allows push' '\n+\tgit --git-dir=dest config receive.maxsize 4k &&\n+\tgit push dest HEAD\n+'\n+\n+test_done\n-- \n2.4.2.752.geeb594a\n"},{"id":"263688","messageId":"20150612185016.GA25214@peff.net","threadId":"39614","inReplyTo":"20150612182045.GA23698@peff.net","subject":"Re: git push keeps writing after server failure","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-06-12T18:50:16Z","receivedAt":"2015-06-12T18:50:16Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jun 12, 2015 at 02:20:45PM -0400, Jeff King wrote:\n\n> > > Notice GitHub prints \"remote: fatal: pack exceeds maximum allowed\n> > > size\". That interrupted my \"Writing objects\" progress meter, and then\n> > > git push just kept going and wrote really really fast (170 MiB/s!)\n> > > until the entire pack was sent.\n> > \n> > Sounds like it's writing to a closed fd, then. Which makes sense; I\n> > think we should hang up the socket after writing the \"fatal\" message\n> > above.\n> \n> For reference, here's the patch implementing the max-size check on the\n> server. It's on my long list of patches to clean up and send to the\n> list; I never did this one because of the unpack-objects caveat\n> mentioned below.\n\nI did a little more digging on this.\n\nWith the max-size patch, we seem to reliably notice the problem and die\nof EPIPE (you can bump receive.maxsize to something reasonable like 1m).\nPushing to GitHub[1], though, sometimes dies and sometimes ends up\npushing the whole pack over the ssh session. It seems racy.\n\nI've confirmed in both cases that the receive-pack process dies on our\nserver. So presumably the problem is in between; it might be an ssh\nweirdness, or it might be a problem with our proxy layer. I'll open an\nissue internally to look at that.\n\n-Peff\n\n[1] I can tweak the max-size on a per-repo basis, which is how I did my\n    testing without waiting for 2G to transfer. If anybody is interested\n    in diagnosing the client side of this, I am happy to drop the\n    max-size on a test repo for you. But AFAICT it is not a client\n    problem at all.\n"}]}