I think the patch handles that by checking that a) the call was done in blocking mode and, more importantly, b) the brigade is empty
So, a TIMEUP happening with a non-empty brigade would still trigger the existing behaviour. It is the specific case where a blocking read returns nothing but the timeout that I am interested in. For synchronous MPMs this would keep the server responsive to graceful restarts/shutdowns even in the face of long KeepAlive settings. Currently, a dying child will simply hang while waiting to a KeepAlive timer to expire. At least, the HTTP/2 code exhibits that. I think reqtimeout does the same. But I may have overlooked something... -Stefan > Am 21.01.2016 um 13:29 schrieb Plüm, Rüdiger, Vodafone Group > <[email protected]>: > > Sorry for being devils advocate, but I have no time to check in depth > currently on my own. So > two points worth checking: > > If the consumer is not prepared to handle TIMEOUTS, how do we ensure that the > possible remaining content of inctx->bb > > 1. is not creating a memory leak. > 2. is not delivered to the next consumer (some kind of request splitting). > > As said no time to investigate deeper on my own. Maybe needless fears on my > side. > > Regards > > Rüdiger > >> -----Ursprüngliche Nachricht----- >> Von: Stefan Eissing [mailto:[email protected]] >> Gesendet: Donnerstag, 21. Januar 2016 11:52 >> An: [email protected] >> Betreff: Re: socket timeouts >> >> However seems to do it. Anyone for/against applying it? >> >> =================================================================== >> --- modules/ssl/ssl_engine_io.c (revision 1725894) >> +++ modules/ssl/ssl_engine_io.c (working copy) >> @@ -506,6 +506,12 @@ >> return -1; >> } >> >> + if (block == APR_BLOCK_READ >> + && APR_STATUS_IS_TIMEUP(inctx->rc) >> + && APR_BRIGADE_EMPTY(inctx->bb)) { >> + /* don't give up, just return the timeout */ >> + return -1; >> + } >> if (inctx->rc != APR_SUCCESS) { >> /* Unexpected errors discard the brigade */ >> >>> Am 20.01.2016 um 21:50 schrieb Yann Ylavic <[email protected]>: >>> >>> On Tue, Jan 19, 2016 at 4:52 PM, Stefan Eissing >>> <[email protected]> wrote: >>>> >>>> I experimented with setting socket timeouts to 1 second during >>>> HTTP/2's keepalive reading and closing the connection after n such >>>> TIMEUP returns. That works nicely on cleartext connections, but >>>> https:// return APR_EOF on reads after the first APR_ETIMEUP. Am I >>>> seeing ghosts or is that something in SSL that regards timed-out >>>> reads as basically broken? >>> >>> Maybe mod_ssl could preserve downstream status when it's already an >> error? >>> >>> Index: modules/ssl/ssl_engine_io.c >>> =================================================================== >>> --- modules/ssl/ssl_engine_io.c (revision 1725250) >>> +++ modules/ssl/ssl_engine_io.c (working copy) >>> @@ -679,7 +679,7 @@ static apr_status_t ssl_io_input_read(bio_filter_i >>> if (*len > 0) { >>> inctx->rc = APR_SUCCESS; >>> } >>> - else { >>> + else if (inctx->rc == APR_SUCCESS) { >>> inctx->rc = APR_EOF; >>> } >>> break; >>> -- >
