Join GitHub today
GitHub is home to over 28 million developers working together to host and review code, manage projects, and build software together.Sign up
IPRO recorder crashes on corrupt brigades with multiple EOS buckets. #1191
We are running last mod pagespped on
in vhost, there is local configuration
Hi thanks for the detailed traceback. Could you correlate this failure
One other thing, can you tell us what other modules you have installed?
This looks like something we should be able to reproduce and fix. I just
On Thu, Nov 19, 2015 at 10:16 AM, mfiala firstname.lastname@example.org wrote:
I have created testing virtual host in apache. I have tested vhost via benchmark tool siege:
siege -v -i -d 1 -c 8 -t 10s -f urls.txt
urls.txt see attachment
Thanks for the config info and url list. It looks like the only non-default module you have besides pagespeed is fastcgi; is that right?
The stack-trace you gave me points to a bug in the code that is trivial to fix, and I have attached a source-patch that you could apply and rebuild. The reason that problem hasn't showed up in the past is, I think, because there has to be some other bug upstream of PageSpeed to trigger it. Specifically, apache transmits response-bodies between filters as bucket-brigades, and I think for us to run into our bug we have to get a corrupt brigade (two EOS-markers) from an upstream filter. It might be sufficient to just fix the problem in mod_pagespeed, but it would be better to fix the problem at the source, if we can figure out what it is.
That's why I was hoping you could correlate the crash against your access log, but that might be hard. But if you can, please apply this patch and see if it solves the problem:
diff --git a/pagespeed/apache/mod_instaweb.cc b/pagespeed/apache/mod_instaweb.cc index c9d6686..b697558 100644 --- a/pagespeed/apache/mod_instaweb.cc +++ b/pagespeed/apache/mod_instaweb.cc @@ -828,6 +828,8 @@ apr_status_t instaweb_in_place_check_headers_filter(ap_filter_t* filter, // Deletes recorder recorder->DoneAndSetHeaders(&response_headers, !request->connection->aborted); + filter->ctx = NULL; + break; } }
added a commit
Nov 20, 2015
I checked in a potential fix to trunk here, which is slightly different than the proposed patch above: 4846786
But if you can, I'd love for you to try it, preferably using this patch instead (identical to the commit):
If this fixes the problem we should consider doing a 1.9 patch to resolve, however we will soon release 1.10 which will have this fix in it.
I was not able to reproduce the problem, it looks that your patch has fixed the problem. Thank you Joshua. We will use the modified version of pagespeed in production. If something goes wrong, I will contact you. Are you going to add this patch to next version of pagespeed? if so, when?