{"thread":{"id":"59138","subject":"t5559 breaks with apache 2.4.55","startedAt":"2023-01-22T08:00:39Z","lastAt":"2023-02-11T03:00:21Z","messageCount":8,"participants":["Jeff King","Todd Zullinger","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"470874","messageId":"Y8ztIqYgVCPILJlO@coredump.intra.peff.net","threadId":"59138","inReplyTo":null,"subject":"t5559 breaks with apache 2.4.55","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-01-22T08:00:34Z","receivedAt":"2023-01-22T08:00:39Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"I noticed that the test suite now fails after upgrading from apache\n2.4.54 to 2.4.55 (the latter of which just hit debian unstable). The\nproblem is with the http2 tests, specifically t5559.30, where we send a\nlarge fetch negotiation over http2. The output from curl is (including\nsome bits of tracing):\n\n == Info: Received 101, Switching to HTTP/2\n == Info: Using HTTP2, server supports multiplexing\n == Info: Copying HTTP/2 data in stream buffer to connection buffer after upgrade: len=0\n == Info: Closing connection 1\n error: RPC failed; HTTP 101 curl 16 Error in the HTTP2 framing layer\n\nBisecting within apache's Git repo, the culprit is their 9767274b88,\nwhich says:\n\n mod_http2: version 2.0.10 of the module, synchronizing changes\n with the gitgub version. This is a partial rewrite of how connections\n and streams are handled.\n\nwhich seems like a plausible source. But the diff is 8000 lines. It may\nbe possible to bisect within the mod_http2 source itself, but I haven't\ntried it yet.\n\nIt's also not 100% clear that it's an apache bug. We could be doing\nsomething weird with git-http-backend, or curl might be doing something\nwrong. Though I tend to doubt it, given the simplicity of the CGI\ninterface on the server side and the fact that curl was working reliably\nwith older versions of apache.\n\nSo I haven't reported the bug further yet. But I thought I'd post this\nhere before anybody else wastes time digging in the same hole.\n\n-Peff\n"},{"id":"470885","messageId":"Y81lQwG85+Skujja@pobox.com","threadId":"59138","inReplyTo":"Y8ztIqYgVCPILJlO@coredump.intra.peff.net","subject":"Re: t5559 breaks with apache 2.4.55","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2023-01-22T16:33:07Z","receivedAt":"2023-01-22T16:38:55Z","isPatch":false,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Jeff King wrote:\n> I noticed that the test suite now fails after upgrading from apache\n> 2.4.54 to 2.4.55 (the latter of which just hit debian unstable). The\n> problem is with the http2 tests, specifically t5559.30, where we send a\n> large fetch negotiation over http2. The output from curl is (including\n> some bits of tracing):\n> \n> == Info: Received 101, Switching to HTTP/2\n> == Info: Using HTTP2, server supports multiplexing\n> == Info: Copying HTTP/2 data in stream buffer to connection buffer after upgrade: len=0\n> == Info: Closing connection 1\n> error: RPC failed; HTTP 101 curl 16 Error in the HTTP2 framing layer\n> \n> Bisecting within apache's Git repo, the culprit is their 9767274b88,\n> which says:\n> \n> mod_http2: version 2.0.10 of the module, synchronizing changes\n> with the gitgub version. This is a partial rewrite of how connections\n> and streams are handled.\n> \n> which seems like a plausible source. But the diff is 8000 lines. It may\n> be possible to bisect within the mod_http2 source itself, but I haven't\n> tried it yet.\n> \n> It's also not 100% clear that it's an apache bug. We could be doing\n> something weird with git-http-backend, or curl might be doing something\n> wrong. Though I tend to doubt it, given the simplicity of the CGI\n> interface on the server side and the fact that curl was working reliably\n> with older versions of apache.\n> \n> So I haven't reported the bug further yet. But I thought I'd post this\n> here before anybody else wastes time digging in the same hole.\n\nFWIW, I think this is the same issue we discussed about 2\nmonths back, in ¹.\n\nI haven't done much else with it since then. It's almost\nsurely either an apache httpd/mod_http2 or curl issue. If I\nhad to bet, I'd say mod_http2. (But then, it could be curl\nand just has yet to be exposed widely because not many are\nusing the current mod_http2 code.)\n\n¹ https://lore.kernel.org/git/Y4fUntdlc1mqwad5@pobox.com/\n\n-- \nTodd\n"},{"id":"471065","messageId":"Y9Jmfg/jlSszVep4@coredump.intra.peff.net","threadId":"59138","inReplyTo":"Y81lQwG85+Skujja@pobox.com","subject":"Re: t5559 breaks with apache 2.4.55","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-01-26T11:39:42Z","receivedAt":"2023-01-26T11:39:58Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jan 22, 2023 at 11:33:07AM -0500, Todd Zullinger wrote:\n\n> > So I haven't reported the bug further yet. But I thought I'd post this\n> > here before anybody else wastes time digging in the same hole.\n> \n> FWIW, I think this is the same issue we discussed about 2\n> months back, in ¹.\n> \n> I haven't done much else with it since then. It's almost\n> surely either an apache httpd/mod_http2 or curl issue. If I\n> had to bet, I'd say mod_http2. (But then, it could be curl\n> and just has yet to be exposed widely because not many are\n> using the current mod_http2 code.)\n> \n> ¹ https://lore.kernel.org/git/Y4fUntdlc1mqwad5@pobox.com/\n\nAh, I somehow completely forgot about that issue. Despite being one of\nthe two participants on the thread.\n\nYeah, after seeing that, I'm quite sure this is a mod_http2 issue. It\nwould be nice to bisect within the mod_http2 history to find the\nculprit, but I'd first have to figure out how to build standalone apache\nmodules. ;)\n\nI may try to poke at it later if I have time. It might also be worth\nsubmitting a bug report to the mod_http2 folks. I'd hope to have a more\ncompact reproduction, but it does at least seem to fail reliably for me\n(not even racily).\n\n-Peff\n"},{"id":"471121","messageId":"Y9YTsF4a8lL7uHm+@coredump.intra.peff.net","threadId":"59138","inReplyTo":"Y9Jmfg/jlSszVep4@coredump.intra.peff.net","subject":"Re: t5559 breaks with apache 2.4.55","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-01-29T06:35:28Z","receivedAt":"2023-01-29T06:35:36Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 26, 2023 at 06:39:42AM -0500, Jeff King wrote:\n\n> Yeah, after seeing that, I'm quite sure this is a mod_http2 issue. It\n> would be nice to bisect within the mod_http2 history to find the\n> culprit, but I'd first have to figure out how to build standalone apache\n> modules. ;)\n\nThis turned out to be quite painless, so I bisected using the source at:\n\n https://github.com/icing/mod_h2\n\nUnfortunately, it's not super helpful for identifying the problem. The\nculprit turns out to be 16ffed9692b, which has a 450-line diff. The\ncommit message is:\n\n * refactored stream response handling to reflect the different phases\n (response/data/trailers) more clearly and help resolving cpu busy loops.\n\nBut that does at least give me more confidence that the bug is in\nmod_http2, and isn't, say, some intentional behavior change there that\nhappens to trigger a bug in curl.\n\nI opened an issue here: https://github.com/icing/mod_h2/issues/243\n\nSo we'll see if that helps.\n\nI re-read your earlier thread, and the version problems you have don't\nquite line up. I think you were having issues with mod_http2 2.0.9, but\nthe older version (1.5.19) worked. I was OK with 2.0.9, but upgrading to\n2.0.10 broke things. I'd hate to have to disable t5559 by default; it\ndoes catch some cases that would trigger in the real-world (since real\nsites like GitHub are using http2). But if it's too unreliable in\npractice, that might be the path of least resistance.\n\n-Peff\n"},{"id":"471880","messageId":"xmqq357ems2h.fsf@gitster.g","threadId":"59138","inReplyTo":"Y9YTsF4a8lL7uHm+@coredump.intra.peff.net","subject":"Re: t5559 breaks with apache 2.4.55","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-02-09T21:45:10Z","receivedAt":"2023-02-09T21:45:15Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> ...\n> But that does at least give me more confidence that the bug is in\n> mod_http2, and isn't, say, some intentional behavior change there that\n> happens to trigger a bug in curl.\n>\n> I opened an issue here: https://github.com/icing/mod_h2/issues/243\n>\n> So we'll see if that helps.\n\nThanks. I've seen that the above issue may have redirected the\ninvestigation to cURL, and over time, I expect taht this will start\nto trigger in more environments (as Apache 2.4.55 and mod_h2 2.0.10\npropagates) before it fixes itself (as fixed versions of these\nthings we use in our tests percolates down).\n\nIn the meantime, perhaps we should punt with a patch like this?\n\n\n------------ >8 ------------\nSubject: [PATCH] t5559: skip a known-to-be-broken test\n\nt5559 runs the same set of tests as t5551 under HTTP/2 but one of\nthem started failing with Apache 2.4.55 and mod_h2 2.0.10 (but not\nwith HTTP/1.1). Newer mod_h2 is known to have fixed the issue.\n\nIn the meantime, skip the test.\n\nSigned-off-by: Junio C Hamano \n---\n t/t5551-http-fetch-smart.sh | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git c/t/t5551-http-fetch-smart.sh w/t/t5551-http-fetch-smart.sh\nindex bc0719a4fc..52159e8b6f 100755\n--- c/t/t5551-http-fetch-smart.sh\n+++ w/t/t5551-http-fetch-smart.sh\n@@ -350,7 +350,8 @@ test_expect_success CMDLINE_LIMIT \\\n \t)\n '\n \n-test_expect_success 'large fetch-pack requests can be sent using chunked encoding' '\n+# https://github.com/icing/mod_h2/issues/243\n+test_expect_success !HTTP2 'large fetch-pack requests can be sent using chunked encoding' '\n \tGIT_TRACE_CURL=true git -c http.postbuffer=65536 \\\n \t\tclone --bare \"$HTTPD_URL/smart/repo.git\" split.git 2>err &&\n \t{\n\n"},{"id":"471961","messageId":"Y+b7bTs2vdXAe/ky@coredump.intra.peff.net","threadId":"59138","inReplyTo":"xmqq357ems2h.fsf@gitster.g","subject":"Re: t5559 breaks with apache 2.4.55","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-02-11T02:20:29Z","receivedAt":"2023-02-11T02:20:35Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 09, 2023 at 01:45:10PM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > ...\n> > But that does at least give me more confidence that the bug is in\n> > mod_http2, and isn't, say, some intentional behavior change there that\n> > happens to trigger a bug in curl.\n> >\n> > I opened an issue here: https://github.com/icing/mod_h2/issues/243\n> >\n> > So we'll see if that helps.\n> \n> Thanks. I've seen that the above issue may have redirected the\n> investigation to cURL, and over time, I expect taht this will start\n> to trigger in more environments (as Apache 2.4.55 and mod_h2 2.0.10\n> propagates) before it fixes itself (as fixed versions of these\n> things we use in our tests percolates down).\n\nYeah, I am worried that this will bite other people, though it's not\nclear to me yet how common the problematic version will be. But...\n\n> In the meantime, perhaps we should punt with a patch like this?\n> \n> \n> ------------ >8 ------------\n> Subject: [PATCH] t5559: skip a known-to-be-broken test\n> \n> t5559 runs the same set of tests as t5551 under HTTP/2 but one of\n> them started failing with Apache 2.4.55 and mod_h2 2.0.10 (but not\n> with HTTP/1.1). Newer mod_h2 is known to have fixed the issue.\n> \n> In the meantime, skip the test.\n\nI'm not sure this is really sufficient. As I was running t5559 over and\nover to test various versions, I noticed a few other cases that seemed\nto fail, some of them racily. So I'm a bit worried that the problem may\nbe more extensive, and it is only that this particular test happened to\ntrigger it reliably.\n\nIn which case the right solution may be ditching t5559, or at the very\nleast adding a knob or version check to disable it.\n\nSo I'm tempted to just punt for now and see how often this bites people\nin the real world. And if it does become a problem, we'll have more data\non what exactly should be changed.\n\n-Peff\n"},{"id":"471962","messageId":"xmqqilg9exq4.fsf@gitster.g","threadId":"59138","inReplyTo":"Y+b7bTs2vdXAe/ky@coredump.intra.peff.net","subject":"Re: t5559 breaks with apache 2.4.55","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-02-11T02:34:43Z","receivedAt":"2023-02-11T02:34:47Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I'm not sure this is really sufficient. As I was running t5559 over and\n> over to test various versions, I noticed a few other cases that seemed\n> to fail, some of them racily. So I'm a bit worried that the problem may\n> be more extensive, and it is only that this particular test happened to\n> trigger it reliably.\n\nYeah, I've seen not just t5559.30 (which is what the patch was\nabout) reliably fail, but t5559.25 fail only from time to time.\nThere may be others.\n\n> In which case the right solution may be ditching t5559, or at the very\n> least adding a knob or version check to disable it.\n\nYup. Do we have an example of checking versions of mod_frob being\nused from within our test suite? I didn't locate one.\n\n> So I'm tempted to just punt for now and see how often this bites people\n> in the real world. And if it does become a problem, we'll have more data\n> on what exactly should be changed.\n\nOK, works for me.\n\nThanks.\n\n"},{"id":"471967","messageId":"Y+cEwLjdgThXdkrw@coredump.intra.peff.net","threadId":"59138","inReplyTo":"xmqqilg9exq4.fsf@gitster.g","subject":"Re: t5559 breaks with apache 2.4.55","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-02-11T03:00:16Z","receivedAt":"2023-02-11T03:00:21Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 10, 2023 at 06:34:43PM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > I'm not sure this is really sufficient. As I was running t5559 over and\n> > over to test various versions, I noticed a few other cases that seemed\n> > to fail, some of them racily. So I'm a bit worried that the problem may\n> > be more extensive, and it is only that this particular test happened to\n> > trigger it reliably.\n> \n> Yeah, I've seen not just t5559.30 (which is what the patch was\n> about) reliably fail, but t5559.25 fail only from time to time.\n> There may be others.\n\nAh, then if it's not just me, maybe it is time to take action. :)\n\nI'm not sure if we have a complete list of good/bad versions, though. I\nknow 2.0.10 was bad, and 2.0.9 seemed to be OK for me. 2.0.12 passes for\nme in regular use, but --stress seems to quickly barf. But it does for\nv2.0.9, too.\n\n> > In which case the right solution may be ditching t5559, or at the very\n> > least adding a knob or version check to disable it.\n> \n> Yup. Do we have an example of checking versions of mod_frob being\n> used from within our test suite? I didn't locate one.\n\nNo, I don't think we do. The answers on stack overflow suggest using\n\"strings\" on the .so file. Not only is that gross, it doesn't seem to\nwork in this case (I find \"mod_http2 (v%s)\", which is useless).\n\nWe could check the outer apache version with \"apachectl -v\", which is\nenough for Debian, but I think Todd had cases where the system was using\na separate version of the mod, not the one that ships with apache\nitself.\n\nI didn't see anything with IfVersion or IfModule that would let us\ndepend on the module version. It does look like we can probably crank\nthe LogLevel up and then grep for \"mod_http2 (v[0-9.]+\" in the log after\nstarting the server. That would let us bail before actually running any\ntests.\n\nWe can be a bit cavalier here with technique, I think, because the worst\ncase is that we say \"this is not a known-good version\" and just skip the\nhttp/2 tests.\n\n> > So I'm tempted to just punt for now and see how often this bites people\n> > in the real world. And if it does become a problem, we'll have more data\n> > on what exactly should be changed.\n> \n> OK, works for me.\n\nWell, I was all set to try a version check, but after seeing the\nrace problems above, it doesn't seem like a good direction, at least not\nyet. So I'm content to sit on my hands for a while longer.\n\n-Peff\n"}]}