* cgit 1.2.3: lighttpd 1.4.57, AlpineLinux [edge]: using cache breaks delivery @ 2020-12-21 16:26 Steffen Nurpmeso 2020-12-21 17:57 ` John Keeping 2021-01-02 18:38 ` Jon DeVree 0 siblings, 2 replies; 14+ messages in thread From: Steffen Nurpmeso @ 2020-12-21 16:26 UTC (permalink / raw) To: cgit Hello. My first post here, so first a "Thank you!", i am using cgit on lighttpd on Linux for half a decade, and it just works (after finally having a usable configuration). I discovered today that cgit no longer delivers pages, and it must have been like that for some time. The server looks show successful delivery, the cgit cache is populated and rotated just correctly, but all cgit delivers is that final error of main() as <div class='error'>Error processing page: Invalid argument (22)</div> (surely a misconfiguration that this is not a real HTML page, i recall it took some time to figure out about about-filter, just "doing it" (<pre>cat(1)</pre>)). If i set "cache-size=0" then the desired page is delivered. Note the cache itself is managed as usual with the default cache-size=1999. The files in the cache are 0600, i thought maybe that was it, but the setting is actively reverted to this mask (i never looked at cgit code except grepping the error). It has always been so, the oldest cache entry was -rw------- 1 lighttpd lighttpd 3023 Mar 18 2018 d2200000 I am pretty sure cgit delivered some weeks ago, the most notable difference is that AlpineLinux switched to Lighttpd 1.4.56 then .57, which seems to have brought tremendous changes under the hood, like HTTP/2 support and OCSP as well as support for all the different TLS libraries, whereas before it only was OpenSSL and compatibles, i think. We have HTTP/2 not enabled yet. Anything i can do about this? --steffen | |Der Kragenbaer, The moon bear, |der holt sich munter he cheerfully and one by one |einen nach dem anderen runter wa.ks himself off |(By Robert Gernhardt) ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: cgit 1.2.3: lighttpd 1.4.57, AlpineLinux [edge]: using cache breaks delivery 2020-12-21 16:26 cgit 1.2.3: lighttpd 1.4.57, AlpineLinux [edge]: using cache breaks delivery Steffen Nurpmeso @ 2020-12-21 17:57 ` John Keeping 2020-12-21 19:31 ` Steffen Nurpmeso 2021-01-02 18:38 ` Jon DeVree 1 sibling, 1 reply; 14+ messages in thread From: John Keeping @ 2020-12-21 17:57 UTC (permalink / raw) To: Steffen Nurpmeso; +Cc: cgit On Mon, Dec 21, 2020 at 05:26:19PM +0100, Steffen Nurpmeso wrote: > I discovered today that cgit no longer delivers pages, and it must > have been like that for some time. The server looks show > successful delivery, the cgit cache is populated and rotated just > correctly, but all cgit delivers is that final error of main() as > > <div class='error'>Error processing page: Invalid argument (22)</div> > > (surely a misconfiguration that this is not a real HTML page, > i recall it took some time to figure out about about-filter, just > "doing it" (<pre>cat(1)</pre>)). > > If i set "cache-size=0" then the desired page is delivered. > Note the cache itself is managed as usual with the default > cache-size=1999. > > The files in the cache are 0600, i thought maybe that was it, but > the setting is actively reverted to this mask (i never looked at > cgit code except grepping the error). It has always been so, the > oldest cache entry was > > -rw------- 1 lighttpd lighttpd 3023 Mar 18 2018 d2200000 > > I am pretty sure cgit delivered some weeks ago, the most notable > difference is that AlpineLinux switched to Lighttpd 1.4.56 then > .57, which seems to have brought tremendous changes under the > hood, like HTTP/2 support and OCSP as well as support for all the > different TLS libraries, whereas before it only was OpenSSL and > compatibles, i think. We have HTTP/2 not enabled yet. > > Anything i can do about this? Have you looked at the log output? This error comes from cgit.c::cmd_main() as a result of cache.c::cache_process() returning an error. It looks like all of the error paths there will write a more detailed error message to stderr. I don't use lighttpd so I don't know whether that output is captured, but normally output written to stderr will end up in a log file. From a quick look at the code, given that the problem seems to be caused by updating the web server, my guess is that you'll see the error: [cgit] error printing cache ...: Invalid argument (22) and this may be caused by sendfile(2) failing due to some difference in how the web server is setting up the output file descriptor. You may want to rebuild CGit without HAVE_LINUX_SENDFILE and see if that works. (If it does, we should add better fallback handling for the case where stdout is not a valid output for sendfile().) John ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: cgit 1.2.3: lighttpd 1.4.57, AlpineLinux [edge]: using cache breaks delivery 2020-12-21 17:57 ` John Keeping @ 2020-12-21 19:31 ` Steffen Nurpmeso 2020-12-21 22:23 ` Steffen Nurpmeso 0 siblings, 1 reply; 14+ messages in thread From: Steffen Nurpmeso @ 2020-12-21 19:31 UTC (permalink / raw) To: John Keeping; +Cc: cgit Hi. John Keeping wrote in <X+DiDgGaPaynnocI@john.keeping.me.uk>: |On Mon, Dec 21, 2020 at 05:26:19PM +0100, Steffen Nurpmeso wrote: |> I discovered today that cgit no longer delivers pages, and it must |> have been like that for some time. The server looks show |> successful delivery, the cgit cache is populated and rotated just |> correctly, but all cgit delivers is that final error of main() as |> |> <div class='error'>Error processing page: Invalid argument (22)</div> |> |> (surely a misconfiguration that this is not a real HTML page, |> i recall it took some time to figure out about about-filter, just |> "doing it" (<pre>cat(1)</pre>)). |> |> If i set "cache-size=0" then the desired page is delivered. |> Note the cache itself is managed as usual with the default |> cache-size=1999. |> |> The files in the cache are 0600, i thought maybe that was it, but |> the setting is actively reverted to this mask (i never looked at |> cgit code except grepping the error). It has always been so, the |> oldest cache entry was |> |> -rw------- 1 lighttpd lighttpd 3023 Mar 18 2018 d2200000 |> |> I am pretty sure cgit delivered some weeks ago, the most notable |> difference is that AlpineLinux switched to Lighttpd 1.4.56 then |> .57, which seems to have brought tremendous changes under the |> hood, like HTTP/2 support and OCSP as well as support for all the |> different TLS libraries, whereas before it only was OpenSSL and |> compatibles, i think. We have HTTP/2 not enabled yet. |> |> Anything i can do about this? | |Have you looked at the log output? Yes, sorry, terrible mess above. Monday and no end. - Logs show nothing. I enabled debug.log-request-handling = "enable" but that showed nothing. But wait, there was server.breakagelog that was needed .. as below! |This error comes from cgit.c::cmd_main() as a result of |cache.c::cache_process() returning an error. It looks like all of the Yes. |error paths there will write a more detailed error message to stderr. Unfortunately nothing .. but with breakaglog there is [cgit] error printing cache /var/lib/lighttpd/cgit/b1000000: Invalid argument (22) But the file was generated normally: # ll /var/lib/lighttpd/cgit/b1000000 -rw------- 1 lighttpd lighttpd 23417 Dec 21 20:22 /var/lib/lighttpd/cgit/b1000000 |I don't use lighttpd so I don't know whether that output is captured, |but normally output written to stderr will end up in a log file. Was server.breakagelog i had forgotten about. |>From a quick look at the code, given that the problem seems to be caused |by updating the web server, my guess is that you'll see the error: | | [cgit] error printing cache ...: Invalid argument (22) | |and this may be caused by sendfile(2) failing due to some difference in |how the web server is setting up the output file descriptor. You may |want to rebuild CGit without HAVE_LINUX_SENDFILE and see if that works. I did not know about 'cgi.x-sendfile = "enable"' yet, but setting it does not matter from cgit's point of view. |(If it does, we should add better fallback handling for the case where |stdout is not a valid output for sendfile().) Ok, i will hm do that mirroring the ArchLinux recipe and report back. Thanks! |John --End of <X+DiDgGaPaynnocI@john.keeping.me.uk> --steffen | |Der Kragenbaer, The moon bear, |der holt sich munter he cheerfully and one by one |einen nach dem anderen runter wa.ks himself off |(By Robert Gernhardt) ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: cgit 1.2.3: lighttpd 1.4.57, AlpineLinux [edge]: using cache breaks delivery 2020-12-21 19:31 ` Steffen Nurpmeso @ 2020-12-21 22:23 ` Steffen Nurpmeso 2020-12-21 23:24 ` Steffen Nurpmeso 0 siblings, 1 reply; 14+ messages in thread From: Steffen Nurpmeso @ 2020-12-21 22:23 UTC (permalink / raw) To: John Keeping, cgit Hello. Steffen Nurpmeso wrote in <20201221193127.zbZeP%steffen@sdaoden.eu>: |John Keeping wrote in | <X+DiDgGaPaynnocI@john.keeping.me.uk>: ||On Mon, Dec 21, 2020 at 05:26:19PM +0100, Steffen Nurpmeso wrote: ||> I discovered today that cgit no longer delivers pages, and it must ||> have been like that for some time. The server looks show ||> successful delivery, the cgit cache is populated and rotated just ||> correctly, but all cgit delivers is that final error of main() as ||> ||> <div class='error'>Error processing page: Invalid argument (22)</div> ... ||> I am pretty sure cgit delivered some weeks ago, the most notable ||> difference is that AlpineLinux switched to Lighttpd 1.4.56 then ||> .57, which seems to have brought tremendous changes under the ... |But the file was generated normally: | | # ll /var/lib/lighttpd/cgit/b1000000 | -rw------- 1 lighttpd lighttpd 23417 Dec 21 20:22 /var/lib/lightt\ | pd/cgit/b1000000 ... Slightly resorted: ... ||Have you looked at the log output? | [cgit] error printing cache /var/lib/lighttpd/cgit/b1000000: Invalid \ | argument (22) ... ||and this may be caused by sendfile(2) failing due to some difference in ||how the web server is setting up the output file descriptor. You may ||want to rebuild CGit without HAVE_LINUX_SENDFILE and see if that works. So i build it with #alp-2020:$ diff cgit.mk.orig cgit.mk --- cgit.mk.orig +++ cgit.mk @@ -64,7 +64,7 @@ endif ifdef HAVE_LINUX_SENDFILE - CGIT_CFLAGS += -DHAVE_LINUX_SENDFILE + #CGIT_CFLAGS += -DHAVE_LINUX_SENDFILE endif CGIT_OBJ_NAMES += cgit.o and make NO_LUA=y NO_ICONV=y NO_GETTEXT=y NO_TCLTK=y NO_PERL=1 \ NO_PYTHON=1 NO_SVN_TESTS=y NO_REGEX=NeedsStartEnd prefix=/usr and .. it works. Thank you, i will open an AlpineLinux bug report. And lighttpd. --steffen | |Der Kragenbaer, The moon bear, |der holt sich munter he cheerfully and one by one |einen nach dem anderen runter wa.ks himself off |(By Robert Gernhardt) ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: cgit 1.2.3: lighttpd 1.4.57, AlpineLinux [edge]: using cache breaks delivery 2020-12-21 22:23 ` Steffen Nurpmeso @ 2020-12-21 23:24 ` Steffen Nurpmeso 0 siblings, 0 replies; 14+ messages in thread From: Steffen Nurpmeso @ 2020-12-21 23:24 UTC (permalink / raw) To: John Keeping, cgit Steffen Nurpmeso wrote in <20201221222300.z9Vio%steffen@sdaoden.eu>: |Steffen Nurpmeso wrote in | <20201221193127.zbZeP%steffen@sdaoden.eu>: ||John Keeping wrote in || <X+DiDgGaPaynnocI@john.keeping.me.uk>: |||On Mon, Dec 21, 2020 at 05:26:19PM +0100, Steffen Nurpmeso wrote: |||> I discovered today that cgit no longer delivers pages, and it must |||> have been like that for some time. The server looks show |||> successful delivery, the cgit cache is populated and rotated just |||> correctly, but all cgit delivers is that final error of main() as |||> |||> <div class='error'>Error processing page: Invalid argument (22)</div> ... |||and this may be caused by sendfile(2) failing due to some difference in |||how the web server is setting up the output file descriptor. You may |||want to rebuild CGit without HAVE_LINUX_SENDFILE and see if that works. | |So i build it with ... | - CGIT_CFLAGS += -DHAVE_LINUX_SENDFILE | + #CGIT_CFLAGS += -DHAVE_LINUX_SENDFILE ... |and | | make NO_LUA=y NO_ICONV=y NO_GETTEXT=y NO_TCLTK=y NO_PERL=1 \ | NO_PYTHON=1 NO_SVN_TESTS=y NO_REGEX=NeedsStartEnd prefix=/usr | |and .. it works. | |Thank you, i will open an AlpineLinux bug report. And lighttpd. Did that. Though redmine lighttpd threw me out after password reset, i posted in #lighttpd irc.freenode.net a few minutes ago. FreeBSD also has a sendfile by the way, had that already twenty years ago, and worked great! Slightly different semantics though. But doable (by then). The guys from nginx strive for tweaking the last little thing out of that btw. Just last week iirc they added NUMA specific ioctls to be able to gain more locality. I think i will go again soon, now that this is worked out :) Thanks, Ciao and greetings from Germany, --steffen | |Der Kragenbaer, The moon bear, |der holt sich munter he cheerfully and one by one |einen nach dem anderen runter wa.ks himself off |(By Robert Gernhardt) ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: cgit 1.2.3: lighttpd 1.4.57, AlpineLinux [edge]: using cache breaks delivery 2020-12-21 16:26 cgit 1.2.3: lighttpd 1.4.57, AlpineLinux [edge]: using cache breaks delivery Steffen Nurpmeso 2020-12-21 17:57 ` John Keeping @ 2021-01-02 18:38 ` Jon DeVree 2021-01-15 18:01 ` Jon DeVree 1 sibling, 1 reply; 14+ messages in thread From: Jon DeVree @ 2021-01-02 18:38 UTC (permalink / raw) To: cgit I think that this is a bug in the 5.10 kernel. I just hit it on a previously working cgit host by upgrading the kernel from 5.9.15 to 5.10.4. Downgrading the kernel back to 5.9.15 fixes it. I'm guessing it was broken by 36e2c7421f02 ("fs: don't allow splice read/write without explicit ops") because if I crank up kernel logging I get: splice write not supported for file (pid: 2522 comm: cgit.cgi) -- Jon Doge Wrangler X(7): A program for managing terminal windows. See also screen(1) and tmux(1). ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: cgit 1.2.3: lighttpd 1.4.57, AlpineLinux [edge]: using cache breaks delivery 2021-01-02 18:38 ` Jon DeVree @ 2021-01-15 18:01 ` Jon DeVree 2021-01-15 21:51 ` Konstantin Ryabitsev 0 siblings, 1 reply; 14+ messages in thread From: Jon DeVree @ 2021-01-15 18:01 UTC (permalink / raw) To: cgit On Sat, Jan 02, 2021 at 13:38:39 -0500, Jon DeVree wrote: > I think that this is a bug in the 5.10 kernel. > I reported this upstream. https://bugzilla.kernel.org/show_bug.cgi?id=211217 -- Jon Doge Wrangler X(7): A program for managing terminal windows. See also screen(1) and tmux(1). ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: cgit 1.2.3: lighttpd 1.4.57, AlpineLinux [edge]: using cache breaks delivery 2021-01-15 18:01 ` Jon DeVree @ 2021-01-15 21:51 ` Konstantin Ryabitsev 2021-01-15 22:41 ` Jon DeVree 0 siblings, 1 reply; 14+ messages in thread From: Konstantin Ryabitsev @ 2021-01-15 21:51 UTC (permalink / raw) To: cgit On Fri, Jan 15, 2021 at 01:01:02PM -0500, Jon DeVree wrote: > On Sat, Jan 02, 2021 at 13:38:39 -0500, Jon DeVree wrote: > > I think that this is a bug in the 5.10 kernel. > > > > I reported this upstream. > > https://bugzilla.kernel.org/show_bug.cgi?id=211217 So, bugzilla.k.o is not *really* upstream, because not all components use it. I know this is dumb, but I'm not in the right situation to fix it (see this discussion here: https://lore.kernel.org/linux-doc/20210110121033.130504-1-linux@leemhuis.info/) Going through bug backlinks, there's a claim that 5.10.4 fixes the problem -- is that accurate, or is the bug still present? -K ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: cgit 1.2.3: lighttpd 1.4.57, AlpineLinux [edge]: using cache breaks delivery 2021-01-15 21:51 ` Konstantin Ryabitsev @ 2021-01-15 22:41 ` Jon DeVree 0 siblings, 0 replies; 14+ messages in thread From: Jon DeVree @ 2021-01-15 22:41 UTC (permalink / raw) To: cgit On Fri, Jan 15, 2021 at 16:51:10 -0500, Konstantin Ryabitsev wrote: > Going through bug backlinks, there's a claim that 5.10.4 fixes the problem -- > is that accurate, or is the bug still present? > 5.10.4 was the first 5.10 kernel uploaded to Debian Sid which is how I discovered the issue. It was still broken on the vanilla 5.10.7 kernel that I installed this morning to see if it was fixed yet. -- Jon Doge Wrangler X(7): A program for managing terminal windows. See also screen(1) and tmux(1). ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: cgit 1.2.3: lighttpd 1.4.57, AlpineLinux [edge]: using cache breaks delivery @ 2020-12-22 6:12 gs-cgit-lists.zx2c4.com 2020-12-22 13:55 ` Steffen Nurpmeso 0 siblings, 1 reply; 14+ messages in thread From: gs-cgit-lists.zx2c4.com @ 2020-12-22 6:12 UTC (permalink / raw) To: cgit >Steffen Nurpmeso wrote in > <20201221193127.zbZeP%steffen at sdaoden.eu>: > |John Keeping wrote in > | <X+DiDgGaPaynnocI at john.keeping.me.uk>: > ||On Mon, Dec 21, 2020 at 05:26:19PM +0100, Steffen Nurpmeso wrote: > ||> I discovered today that cgit no longer delivers pages, and it must > ||> have been like that for some time. The server looks show > ||> successful delivery, the cgit cache is populated and rotated just > ||> correctly, but all cgit delivers is that final error of main() as > ||> > ||> <div class='error'>Error processing page: Invalid argument (22)</div> > ... > ||> I am pretty sure cgit delivered some weeks ago, the most notable > ||> difference is that AlpineLinux switched to Lighttpd 1.4.56 then > ||> .57, which seems to have brought tremendous changes under the > ... > |But the file was generated normally: > | > | # ll /var/lib/lighttpd/cgit/b1000000 > | -rw------- 1 lighttpd lighttpd 23417 Dec 21 20:22 /var/lib/lighttpd/cgit/b1000000 > ... > >Slightly resorted: > > ... > ||Have you looked at the log output? > | [cgit] error printing cache /var/lib/lighttpd/cgit/b1000000: Invalid argument (22) > ... > ||and this may be caused by sendfile(2) failing due to some difference in > ||how the web server is setting up the output file descriptor. You may > ||want to rebuild CGit without HAVE_LINUX_SENDFILE and see if that works. > >So i build it with > > #alp-2020:$ diff cgit.mk.orig cgit.mk > --- cgit.mk.orig > +++ cgit.mk > @@ -64,7 +64,7 @@ > endif > > ifdef HAVE_LINUX_SENDFILE > - CGIT_CFLAGS += -DHAVE_LINUX_SENDFILE > + #CGIT_CFLAGS += -DHAVE_LINUX_SENDFILE > endif > > CGIT_OBJ_NAMES += cgit.o > >and > > make NO_LUA=y NO_ICONV=y NO_GETTEXT=y NO_TCLTK=y NO_PERL=1 \ > NO_PYTHON=1 NO_SVN_TESTS=y NO_REGEX=NeedsStartEnd prefix=/usr > >and .. it works. > >Thank you, i will open an AlpineLinux bug report. And lighttpd. > >--steffen I would like to propose an alternative and more portable solution: cgit cache should fallback to lseek, xread, xwrite if sendfile fails. Even if the kernel supports sendfile() and cgit is built with HAVE_LINUX_SENDFILE defined, certain filesystem types might not support sendfile() operations. The following patch falls back to lseek, xread, xwrite if the *initial* call to sendfile() fails. diff --git a/cache.c b/cache.c index 2c70be7..7fbab66 100644 --- a/cache.c +++ b/cache.c @@ -85,6 +85,7 @@ static int close_slot(struct cache_slot *slot) /* Print the content of the active cache slot (but skip the key). */ static int print_slot(struct cache_slot *slot) { + ssize_t i, j; #ifdef HAVE_LINUX_SENDFILE off_t start_off; int ret; @@ -97,12 +98,13 @@ static int print_slot(struct cache_slot *slot) if (ret < 0) { if (errno == EAGAIN || errno == EINTR) continue; + if (start_off == slot->keylen + 1) + break; return errno; } return 0; } while (1); -#else - ssize_t i, j; +#endif i = lseek(slot->cache_fd, slot->keylen + 1, SEEK_SET); if (i != slot->keylen + 1) @@ -118,7 +120,6 @@ static int print_slot(struct cache_slot *slot) return errno; else return 0; -#endif } /* Check if the slot has expired */ ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: cgit 1.2.3: lighttpd 1.4.57, AlpineLinux [edge]: using cache breaks delivery 2020-12-22 6:12 gs-cgit-lists.zx2c4.com @ 2020-12-22 13:55 ` Steffen Nurpmeso 2020-12-22 15:09 ` Steffen Nurpmeso 0 siblings, 1 reply; 14+ messages in thread From: Steffen Nurpmeso @ 2020-12-22 13:55 UTC (permalink / raw) To: gs-cgit-lists.zx2c4.com; +Cc: cgit gs-cgit-lists.zx2c4.com@gluelogic.com wrote in <20201222061206.GA54419@xps13>: |>Steffen Nurpmeso wrote in |> <20201221193127.zbZeP%steffen at sdaoden.eu>: |>|John Keeping wrote in |>| <X+DiDgGaPaynnocI at john.keeping.me.uk>: |>||On Mon, Dec 21, 2020 at 05:26:19PM +0100, Steffen Nurpmeso wrote: |>||> I discovered today that cgit no longer delivers pages, and it must |>||> have been like that for some time. The server looks show |>||> successful delivery, the cgit cache is populated and rotated just |>||> correctly, but all cgit delivers is that final error of main() as |>||> |>||> <div class='error'>Error processing page: Invalid argument (22)</div\ |>||> > |> ... |>||> I am pretty sure cgit delivered some weeks ago, the most notable |>||> difference is that AlpineLinux switched to Lighttpd 1.4.56 then |>||> .57, which seems to have brought tremendous changes under the |> ... |>|But the file was generated normally: |>| |>| # ll /var/lib/lighttpd/cgit/b1000000 |>| -rw------- 1 lighttpd lighttpd 23417 Dec 21 20:22 /var/lib/light\ |>| tpd/cgit/b1000000 |> ... |> |>Slightly resorted: |> |> ... |>||Have you looked at the log output? |>| [cgit] error printing cache /var/lib/lighttpd/cgit/b1000000: Invalid \ |>| argument (22) |> ... |>||and this may be caused by sendfile(2) failing due to some difference in |>||how the web server is setting up the output file descriptor. You may |>||want to rebuild CGit without HAVE_LINUX_SENDFILE and see if that works. |> |>So i build it with ... |> - CGIT_CFLAGS += -DHAVE_LINUX_SENDFILE |> + #CGIT_CFLAGS += -DHAVE_LINUX_SENDFILE ... |>and .. it works. |> |>Thank you, i will open an AlpineLinux bug report. And lighttpd. ... |I would like to propose an alternative and more portable solution: | |cgit cache should fallback to lseek, xread, xwrite if sendfile fails. (Yes.) |Even if the kernel supports sendfile() and cgit is built with |HAVE_LINUX_SENDFILE defined, certain filesystem types might not support |sendfile() operations. This is ext3 and the overall system has not changed for years (but following AlpineLinux [edge]). |The following patch falls back to lseek, xread, xwrite if the *initial* |call to sendfile() fails. I will use it this afternoon, and do some more testing on the descriptor (fstat, i don't know) because gstrauss (lighttpd maintainer) is "driving up the walls" because he cannot reproduce the issue it seems. He says nothing changed regarding the descriptor handling, mysterious, maybe a kernel bug, 5.10.1 it is at the moment. |diff --git a/cache.c b/cache.c ... Thanks, and Ciao, --steffen | |Der Kragenbaer, The moon bear, |der holt sich munter he cheerfully and one by one |einen nach dem anderen runter wa.ks himself off |(By Robert Gernhardt) ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: cgit 1.2.3: lighttpd 1.4.57, AlpineLinux [edge]: using cache breaks delivery 2020-12-22 13:55 ` Steffen Nurpmeso @ 2020-12-22 15:09 ` Steffen Nurpmeso 2020-12-22 21:22 ` Steffen Nurpmeso 0 siblings, 1 reply; 14+ messages in thread From: Steffen Nurpmeso @ 2020-12-22 15:09 UTC (permalink / raw) To: gs-cgit-lists.zx2c4.com; +Cc: cgit Hello! Steffen Nurpmeso wrote in <20201222135537.ovyl4%steffen@sdaoden.eu>: |gs-cgit-lists.zx2c4.com@gluelogic.com wrote in | <20201222061206.GA54419@xps13>: ||>Steffen Nurpmeso wrote in ||> <20201221193127.zbZeP%steffen at sdaoden.eu>: ||>|John Keeping wrote in ||>| <X+DiDgGaPaynnocI at john.keeping.me.uk>: ||>||On Mon, Dec 21, 2020 at 05:26:19PM +0100, Steffen Nurpmeso wrote: ||>||> I discovered today that cgit no longer delivers pages, and it must ||>||> have been like that for some time. The server looks show ||>||> successful delivery, the cgit cache is populated and rotated just ||>||> correctly, but all cgit delivers is that final error of main() as ||>||> ||>||> <div class='error'>Error processing page: Invalid argument \ ||>||> (22)</div\ ||>||>> ||> ... ||>||> I am pretty sure cgit delivered some weeks ago, the most notable ||>||> difference is that AlpineLinux switched to Lighttpd 1.4.56 then ||>||> .57, which seems to have brought tremendous changes under the ||> ... ||>|But the file was generated normally: ||>| ||>| # ll /var/lib/lighttpd/cgit/b1000000 ||>| -rw------- 1 lighttpd lighttpd 23417 Dec 21 20:22 /var/lib/ligh\ ||>| t\ ||>| tpd/cgit/b1000000 ||> ... ||> ||>Slightly resorted: ||> ||> ... ||>||Have you looked at the log output? ||>| [cgit] error printing cache /var/lib/lighttpd/cgit/b1000000: Invalid \ ||>| argument (22) ||> ... ||>||and this may be caused by sendfile(2) failing due to some difference in ||>||how the web server is setting up the output file descriptor. You may ||>||want to rebuild CGit without HAVE_LINUX_SENDFILE and see if that works. ||> ||>So i build it with | ... ||> - CGIT_CFLAGS += -DHAVE_LINUX_SENDFILE ||> + #CGIT_CFLAGS += -DHAVE_LINUX_SENDFILE | ... ||>and .. it works. ||> ||>Thank you, i will open an AlpineLinux bug report. And lighttpd. | ... | ||I would like to propose an alternative and more portable solution: || ||cgit cache should fallback to lseek, xread, xwrite if sendfile fails. | |(Yes.) .... diff --git a/cache.c b/cache.c index 2c70be7..7fbab66 100644 --- a/cache.c +++ b/cache.c @@ -85,6 +85,7 @@ static int close_slot(struct cache_slot *slot) /* Print the content of the active cache slot (but skip the key). */ static int print_slot(struct cache_slot *slot) { + ssize_t i, j; #ifdef HAVE_LINUX_SENDFILE off_t start_off; int ret; @@ -97,12 +98,13 @@ static int print_slot(struct cache_slot *slot) if (ret < 0) { if (errno == EAGAIN || errno == EINTR) continue; + if (start_off == slot->keylen + 1) + break; I can confirm this hunk fixes the bug. If i remove it i get the error, with it you are fine. return errno; } return 0; } while (1); -#else - ssize_t i, j; +#endif i = lseek(slot->cache_fd, slot->keylen + 1, SEEK_SET); if (i != slot->keylen + 1) @@ -118,7 +120,6 @@ static int print_slot(struct cache_slot *slot) return errno; else return 0; -#endif } /* Check if the slot has expired */ Thanks :), and ciao from Germany, --steffen | |Der Kragenbaer, The moon bear, |der holt sich munter he cheerfully and one by one |einen nach dem anderen runter wa.ks himself off |(By Robert Gernhardt) ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: cgit 1.2.3: lighttpd 1.4.57, AlpineLinux [edge]: using cache breaks delivery 2020-12-22 15:09 ` Steffen Nurpmeso @ 2020-12-22 21:22 ` Steffen Nurpmeso 2020-12-29 17:04 ` Steffen Nurpmeso 0 siblings, 1 reply; 14+ messages in thread From: Steffen Nurpmeso @ 2020-12-22 21:22 UTC (permalink / raw) To: gs-cgit-lists.zx2c4.com; +Cc: cgit Once more, hey, Steffen Nurpmeso wrote in <20201222150957._aAW5%steffen@sdaoden.eu>: |Steffen Nurpmeso wrote in | <20201222135537.ovyl4%steffen@sdaoden.eu>: ||gs-cgit-lists.zx2c4.com@gluelogic.com wrote in || <20201222061206.GA54419@xps13>: |||>Steffen Nurpmeso wrote in |||> <20201221193127.zbZeP%steffen at sdaoden.eu>: |||>|John Keeping wrote in |||>| <X+DiDgGaPaynnocI at john.keeping.me.uk>: |||>||On Mon, Dec 21, 2020 at 05:26:19PM +0100, Steffen Nurpmeso wrote: |||>||> I discovered today that cgit no longer delivers pages, and it must |||>||> have been like that for some time. The server looks show |||>||> successful delivery, the cgit cache is populated and rotated just |||>||> correctly, but all cgit delivers is that final error of main() as ... |||>||> I am pretty sure cgit delivered some weeks ago, the most notable |||>||> difference is that AlpineLinux switched to Lighttpd 1.4.56 then |||>||> .57, which seems to have brought tremendous changes under the |||> ... |||>|But the file was generated normally: ... |||>||and this may be caused by sendfile(2) failing due to some difference \ |||>||in |||>||how the web server is setting up the output file descriptor. You may ... Looking at lighttpd commit history it seems he now uses splice(2) for CGI if possible. ... |||I would like to propose an alternative and more portable solution: ||| |||cgit cache should fallback to lseek, xread, xwrite if sendfile fails. It would be cool if the sendfile(2) could be avoided via configuration. It seems so wasteful to always have that system-call, context-switch and such. What do you think of (on [master] aka adcc4f822fe11836e5f942fc1ae0f00db4eb8d5f plus the other patch), the following runs on my server VM now and works: diff --git a/cache.c b/cache.c index b09769e..8a66db9 100644 --- a/cache.c +++ b/cache.c @@ -25,6 +25,7 @@ struct cache_slot { const char *key; size_t keylen; + int sndfpok; int ttl; cache_fill_fn fn; int cache_fd; @@ -92,7 +93,7 @@ static int print_slot(struct cache_slot *slot) start_off = slot->keylen + 1; - do { + if (slot->sndfpok) for(;;) { ret = sendfile(STDOUT_FILENO, slot->cache_fd, &start_off, slot->cache_st.st_size - start_off); if (ret < 0) { @@ -103,7 +104,7 @@ static int print_slot(struct cache_slot *slot) return errno; } return 0; - } while (1); + } #endif i = lseek(slot->cache_fd, slot->keylen + 1, SEEK_SET); @@ -350,7 +351,7 @@ static int process_slot(struct cache_slot *slot) /* Print cached content to stdout, generate the content if necessary. */ int cache_process(int size, const char *path, const char *key, int ttl, - cache_fill_fn fn) + cache_fill_fn fn, int sndfpok) { unsigned long hash; int i; @@ -383,6 +384,7 @@ int cache_process(int size, const char *path, const char *key, int ttl, strbuf_addbuf(&lockname, &filename); strbuf_addstr(&lockname, ".lock"); slot.fn = fn; + slot.sndfpok = sndfpok; slot.ttl = ttl; slot.stdout_fd = -1; slot.cache_name = filename.buf; diff --git a/cache.h b/cache.h index 470da4f..3e25daa 100644 --- a/cache.h +++ b/cache.h @@ -17,12 +17,13 @@ typedef void (*cache_fill_fn)(void); * key the key used to lookup cache files * ttl max cache time in seconds for this key * fn content generator function for this key + * sndfpok allowed to use sendfile(2) * * Return value * 0 indicates success, everything else is an error */ extern int cache_process(int size, const char *path, const char *key, int ttl, - cache_fill_fn fn); + cache_fill_fn fn, int sndfpok); /* List info about all cache entries on stdout */ diff --git a/cgit.c b/cgit.c index 08d81a1..684f97a 100644 --- a/cgit.c +++ b/cgit.c @@ -197,6 +197,8 @@ static void config_cb(const char *name, const char *value) ctx.cfg.enable_git_config = atoi(value); else if (!strcmp(name, "max-stats")) ctx.cfg.max_stats = cgit_find_stats_period(value, NULL); + else if (!strcmp(name, "cache-sendfile")) + ctx.cfg.cache_sendfile = atoi(value); else if (!strcmp(name, "cache-size")) ctx.cfg.cache_size = atoi(value); else if (!strcmp(name, "cache-root")) @@ -363,6 +365,7 @@ static void prepare_context(void) { memset(&ctx, 0, sizeof(ctx)); ctx.cfg.agefile = "info/web/last-modified"; + ctx.cfg.cache_sendfile = 1; ctx.cfg.cache_size = 0; ctx.cfg.cache_max_create_time = 5; ctx.cfg.cache_root = CGIT_CACHE_ROOT; @@ -1103,7 +1106,8 @@ int cmd_main(int argc, const char **argv) if (!ctx.env.authenticated || (ctx.env.request_method && !strcmp(ctx.env.request_method, "HEAD"))) ctx.cfg.cache_size = 0; err = cache_process(ctx.cfg.cache_size, ctx.cfg.cache_root, - ctx.qry.raw, ttl, process_request); + ctx.qry.raw, ttl, process_request, + ctx.cfg.cache_sendfile); cgit_cleanup_filters(); if (err) cgit_print_error("Error processing page: %s (%d)", diff --git a/cgit.h b/cgit.h index 69b5c13..21677a6 100644 --- a/cgit.h +++ b/cgit.h @@ -215,6 +215,7 @@ struct cgit_config { char *repository_sort; char *virtual_root; /* Always ends with '/'. */ char *strict_export; + int cache_sendfile; int cache_size; int cache_dynamic_ttl; int cache_max_create_time; diff --git a/cgitrc.5.txt b/cgitrc.5.txt index 33a6a8c..7ea1b81 100644 --- a/cgitrc.5.txt +++ b/cgitrc.5.txt @@ -83,6 +83,10 @@ cache-scanrc-ttl:: of scanning a path for git repositories. See also: "CACHE". Default value: "15". +cache-sendfile:: + Whether sendfile(2) optimization may be used for CGI outputs, when + available. Default value: "1". + case-sensitive-sort:: Sort items in the repo list case sensitively. Default value: "1". See also: repository-sort, section-sort. --steffen | |Der Kragenbaer, The moon bear, |der holt sich munter he cheerfully and one by one |einen nach dem anderen runter wa.ks himself off |(By Robert Gernhardt) ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: cgit 1.2.3: lighttpd 1.4.57, AlpineLinux [edge]: using cache breaks delivery 2020-12-22 21:22 ` Steffen Nurpmeso @ 2020-12-29 17:04 ` Steffen Nurpmeso 0 siblings, 0 replies; 14+ messages in thread From: Steffen Nurpmeso @ 2020-12-29 17:04 UTC (permalink / raw) To: cgit; +Cc: gs-cgit-lists.zx2c4.com Steffen Nurpmeso wrote in <20201222212207.3qb-M%steffen@sdaoden.eu>: |Steffen Nurpmeso wrote in | <20201222150957._aAW5%steffen@sdaoden.eu>: ||Steffen Nurpmeso wrote in || <20201222135537.ovyl4%steffen@sdaoden.eu>: |||gs-cgit-lists.zx2c4.com@gluelogic.com wrote in ||| <20201222061206.GA54419@xps13>: ||||>Steffen Nurpmeso wrote in ||||> <20201221193127.zbZeP%steffen at sdaoden.eu>: ||||>|John Keeping wrote in ||||>| <X+DiDgGaPaynnocI at john.keeping.me.uk>: ||||>||On Mon, Dec 21, 2020 at 05:26:19PM +0100, Steffen Nurpmeso wrote: ||||>||> I discovered today that cgit no longer delivers pages, and it must ||||>||> have been like that for some time. The server looks show ||||>||> successful delivery, the cgit cache is populated and rotated just ||||>||> correctly, but all cgit delivers is that final error of main() as | ... ||||>||> I am pretty sure cgit delivered some weeks ago, the most notable ||||>||> difference is that AlpineLinux switched to Lighttpd 1.4.56 then ||||>||> .57, which seems to have brought tremendous changes under the ||||> ... ||||>|But the file was generated normally: | ... ||||>||and this may be caused by sendfile(2) failing due to some difference \ ||||>||\ ||||>||in ||||>||how the web server is setting up the output file descriptor. You may | ... | |Looking at lighttpd commit history it seems he now uses splice(2) |for CGI if possible. On AlpineLinux with kernel 10.3 the unmodified cgit still fails. I just want to note there is no kernel bugzilla entry regarding sendfile and/or splice. I assume the sendfile(2) failure fallback code path becomes implemented and will unsubscribe again now. Thanks. --steffen | |Der Kragenbaer, The moon bear, |der holt sich munter he cheerfully and one by one |einen nach dem anderen runter wa.ks himself off |(By Robert Gernhardt) ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-01-15 22:41 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-12-21 16:26 cgit 1.2.3: lighttpd 1.4.57, AlpineLinux [edge]: using cache breaks delivery Steffen Nurpmeso 2020-12-21 17:57 ` John Keeping 2020-12-21 19:31 ` Steffen Nurpmeso 2020-12-21 22:23 ` Steffen Nurpmeso 2020-12-21 23:24 ` Steffen Nurpmeso 2021-01-02 18:38 ` Jon DeVree 2021-01-15 18:01 ` Jon DeVree 2021-01-15 21:51 ` Konstantin Ryabitsev 2021-01-15 22:41 ` Jon DeVree 2020-12-22 6:12 gs-cgit-lists.zx2c4.com 2020-12-22 13:55 ` Steffen Nurpmeso 2020-12-22 15:09 ` Steffen Nurpmeso 2020-12-22 21:22 ` Steffen Nurpmeso 2020-12-29 17:04 ` Steffen Nurpmeso
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).