Re: [RFC PATCH v7 1/9] gitweb: Go to DONE_REQUEST rather than DONE_GITWEB in die_error
From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:50:17
Jakub Narebski wrote:
End the request after die_error finishes, rather than exiting gitweb instance
[...]
quoted hunk ↗ jump to hunk
--- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl@@ -1169,6 +1169,7 @@ sub run { run_request(); + DONE_REQUEST: $post_dispatch_hook->() if $post_dispatch_hook; $first_request = 0;@@ -3767,7 +3768,7 @@ EOF
[side note: the "@@ EOF" line above would say "@@ sub die_error {" if
userdiff.c had perl support and gitattributes used it.]
print "</div>\n";
git_footer_html();
- goto DONE_GITWEB
+ goto DONE_REQUEST
unless ($opts{'-error_handler'});This seems to remove the last user of the DONE_GITWEB label. Why not delete the label, too? When die_error is called by CGI::Carp (via handle_errors_html), it does not rearm the error handler afaict. Previously that did not matter because die_error kills gitweb; now should it be set up again? die_error gets called when server load is too high; I wonder whether it is right to go back for another request in that case. A broken per-request (or other) configuration could potentially leave a gitweb process in a broken state, and until now the state would be reset on the first error. I wonder if escape valve would be needed
--- e.g., does the CGI harness take care of starting a new gitwebprocess after every couple hundred requests or so? Aside from those (minor) worries, this patch seems like a good idea.