Re: [RFC PATCH v7 1/9] gitweb: Go to DONE_REQUEST rather than DONE_GITWEB in die_error

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jakub Narebski wrote:

> End the request after die_error finishes, rather than exiting gitweb
> instance
[...]
> --- 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 gitweb
process after every couple hundred requests or so?

Aside from those (minor) worries, this patch seems like a good idea.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]