Re: Detecting client aborts and stream resets
On Wed, May 4, 2016 at 3:44 PM, Michael Kaufmann wrote: > William is right, this is not a good idea. The ->aborted flag should serve >> this purpose of telling anyone interested that this connection is not >> longer delivering. I will make a github release soon where that is working >> and you can test. >> >> > Thank you Stefan! It is now working for stream resets, but it's not yet > working if the client closes the whole TCP connection. > As expected... this is why I pointed out in my first reply that you don't want a single-protocol solution to this puzzle. See my later reply about detecting connection tear-down, patches welcome.
Re: Detecting client aborts and stream resets
William is right, this is not a good idea. The ->aborted flag should serve this purpose of telling anyone interested that this connection is not longer delivering. I will make a github release soon where that is working and you can test. Thank you Stefan! It is now working for stream resets, but it's not yet working if the client closes the whole TCP connection. Regards, Michael
Re: Detecting client aborts and stream resets
On Tue, May 3, 2016 at 2:05 PM, Michael Kaufmann wrote: > Zitat von William A Rowe Jr : > >> >> Nope - an optional function in mod_http2 is too special case, generators >> must remain protocol (socket or other transport) agnostic. >> > > Sure, official Apache modules should not call protocol-dependent hooks, > but it could be a solution for 3rd party modules. Nonsense, third party module designers should be equally aware - when you start meddling in unexpected edge cases, binary ABI won't be maintained in those edge cases. You are always better off relying on the intended design of httpd, unless you want to chase a moving target. Extending this detection with a protocol-agnostic API would be interesting, however an optional function has only a single registered provider, so it would be the wrong interface. You want an interface that would test/report "gone away" irrespective of http1 vs http2 vs future protocols. > In the case of mod_cache'd content, the generator can't quit, it is already >> populating a cache, which means you'll generate an invalid cached object. >> >> But if you knew that the cache module wasn't collecting info, >> r->c->aborted >> tells you if anyone is still listening, right? >> > > Unfortunately not. While the content generator is running (just preparing > the response, it is not reading from the input filters anymore and not > writing to the output filters yet), Apache does not check the connection's > state. > Good point... where/how would we address this for CPU intensive generators? We certainly don't bother in proxy, because proxy is much lighter-weight. The obvious answer to me in a worker or event MPM would be for the user to make a 'notify close?' request to the MPM, which would add the socket with for the POLLHUP event alone after reading the input brigade (the read will signal r->c->aborted already), with the single goal of setting the flag r->c->aborted upon socket close. Very lightweight, but that presumes that a second poll on the same fd for different events (e.g. poll on read or write) elsewhere won't give the kernel conniptions. mod_http2 has enough going on the master connection that detecting the stream close should occur fairly promptly.
OpenSSL Patch requires new treatment of dual certificate configurations with ServerInfoFile
Hi to all. I would like to draw your attention to a new patch for OpenSSL which will ultimately mean that Apache needs to treat dual EC-RSA certificate configurations with server info (currently used only for TLS extension of certificate transparency) differently than until now. Specifically, the patches are https://github.com/openssl/openssl/pull/914 and https://github.com/openssl/openssl/pull/915. They originated from research involving my Apache server configuration (2.4.20 on Ubuntu 16.04, not Apache trunk) and Castaglia's coding of patches. The Apache/OpenSSL bug is described fully here: http://serverfault.com/questions/758482/apache-extension-error (the software I used when I published this Serverfault thread was a bit older than now, but the problem still persists). In particular, see the comment of Castaglia on their answer to the thread for possible new Apache idea of implementation. Maybe the following would be a good approach: After the first certificate-private key pair, accept a ServerInfo Openssl configuration directive which would call SSL_CTX_use_serverinfo_file for that certificate. Then the configuration goes on with the second certificate-private key pair and after that, the second serverinfo file location via Openssl configuration directive (if applicable, that is if the server has dual certificate configuration). So, Apache would need to process each pair and then, if it finds directly below it a serverinfo, call SSL_CTX_use_serverinfo_file for THAT certificate. When a new certificate-key pair is registered, the SSL_CTX_use_serverinfo_file would be called again but for the last certificate only. And a last thing: Let's not only implement this for 2.5 trunk, but as a patch for 2.4, eg 2.4.21. Regards, Jason
Re: core_output, files and setaside
On 04 May 2016, at 3:22 PM, Stefan Eissing wrote: > file_bucket_setaside() currently does not care about the refcount. setaside > affects *all* shared file buckets, wherever they currently reside. So it > moves the contained apr_file_t into the filter deferred pool, eventually > clears that pool and the file is closed via cleanup of the file (not the > bucket). That’s broken, we need to fix that so it works properly. > While dup'ing the file descriptor would work, it seems overly costly in this > case. What is the case? > > The output filter already makes the distinction wether filter->r is set or > not. When filter->r is set, it uses filter->r->pool to setaside buckets it > wants to keep around. This is safe since it knows that some time in the > future, an EOR bucket will come along and cleanup - at the right time. > > HTTP/2 has a similar bucket lifetime case: only now, there are several > requests ongoing and interleaved onto the one master connection. But the > basic assumption still holds: there will be some kind of EOR bucket that > manages the lifetimes of buckets before it correctly. > > But the output filter does not know this and even if, would not know which > pool to setaside which bucket to. That’s expected - during requests, we set aside into the request pool, where requests are one shot and you’re done. During connections however we cannot use the connection pool, because the connection pool lives potentially for a very long time. This is why the deferred pool exists. None of this matters though, buckets should work correctly in both cases. > One way for a generic approach is a new META bucket: POOL_LIFE that carries a > pool or NULL. It's contract is: all buckets that follow me have the lifetime > of my pool (at least) and this holds until another POOL_LIFE bucket comes > along. Pools announced in this way are promised to only disappear after some > kind of EOR or FLUSH has been sent. This breaks the contract of pools - every bucket has a pool, and now there would be a second mechanism that duplicates this first mechanism, and as soon as there is a mismatch we crash. Let’s just fix the original bug - make sure that file buckets behave correctly when setaside+refcount is used at the same time. Regards, Graham —
Re: core_output, files and setaside
> Am 04.05.2016 um 13:49 schrieb Graham Leggett : > > On 04 May 2016, at 11:13 AM, Stefan Eissing > wrote: > >> The problem is not the apr_bucket_destroy(). The file bucket setaside, calls >> apr_file_setaside(), in core_output on a deferred pool, and then core_output >> clears that pool. This invalidates all still existing file buckets using >> that apr_file. > > This scenario should still work properly, it shouldn’t cause anything to > break. > > First off, file buckets need reference counting to make sure that only on the > last removal of the bucket the file is closed (it may already do this, I > haven’t looked, but then if it did do this properly it should work). > > Next, if a file bucket is setaside, but the reference count isn’t one (in > other words, other file buckets exist pointing at the same file descriptor in > other places), and the pool we’re setting aside into isn’t the same or a > child pool, we should dup the file descriptor during the setaside. > > The typical scenario for the deferred pool should be the first scenario above. file_bucket_setaside() currently does not care about the refcount. setaside affects *all* shared file buckets, wherever they currently reside. So it moves the contained apr_file_t into the filter deferred pool, eventually clears that pool and the file is closed via cleanup of the file (not the bucket). While dup'ing the file descriptor would work, it seems overly costly in this case. What is the case? The output filter already makes the distinction wether filter->r is set or not. When filter->r is set, it uses filter->r->pool to setaside buckets it wants to keep around. This is safe since it knows that some time in the future, an EOR bucket will come along and cleanup - at the right time. HTTP/2 has a similar bucket lifetime case: only now, there are several requests ongoing and interleaved onto the one master connection. But the basic assumption still holds: there will be some kind of EOR bucket that manages the lifetimes of buckets before it correctly. But the output filter does not know this and even if, would not know which pool to setaside which bucket to. So. One way for a generic approach is a new META bucket: POOL_LIFE that carries a pool or NULL. It's contract is: all buckets that follow me have the lifetime of my pool (at least) and this holds until another POOL_LIFE bucket comes along. Pools announced in this way are promised to only disappear after some kind of EOR or FLUSH has been sent. Given that requests R1, R2, R3 with pools P1, P2, P3 are under way, a brigade passed to core output would look like this: [PL P1][R1 DATA][R1 DATA][PL P3][R3 DATA][PL P2][R2 EOR][PL P1][R1 DATA][PL NULL]... where the PL buckets would switch the setaside pool used by core_output. if no POOL_LIFE is seen or if the contained pool is NULL, the current defaults (r->pool / deferred) are used. Using pools instead of higher level, protocol specific entities (such as request_rec) should make that flexible enough for different use cases. Thoughts? -Stefan
Re: core_output, files and setaside
On 04 May 2016, at 11:13 AM, Stefan Eissing wrote: > The problem is not the apr_bucket_destroy(). The file bucket setaside, calls > apr_file_setaside(), in core_output on a deferred pool, and then core_output > clears that pool. This invalidates all still existing file buckets using that > apr_file. This scenario should still work properly, it shouldn’t cause anything to break. First off, file buckets need reference counting to make sure that only on the last removal of the bucket the file is closed (it may already do this, I haven’t looked, but then if it did do this properly it should work). Next, if a file bucket is setaside, but the reference count isn’t one (in other words, other file buckets exist pointing at the same file descriptor in other places), and the pool we’re setting aside into isn’t the same or a child pool, we should dup the file descriptor during the setaside. The typical scenario for the deferred pool should be the first scenario above. Regards, Graham —
Re: core_output, files and setaside
> Am 04.05.2016 um 11:09 schrieb Graham Leggett : > > On 04 May 2016, at 10:45 AM, Stefan Eissing > wrote: > >> I have been wrong before...but... >> >> mod_http2 needs to send out a file response: >> 1. it starts with the response body brigade: [file:0-len][eos] >> 2. it sends the first 16K frame by splitting the file bucket: >> -> passing to core output: [heap:frame header][file:0-16k] >> -> remaining body: [file:16K-len][eos] >> 3. core_output decides to setaside: >> -> setaside (deferred pool): [heap:frame header][file:0-16k] >> -> remaining body: [file:16K-len][eos] >> 4. core_output sends and, sometimes, clears the deferred pool >> -> which closes the file descriptor >> 5. next 16K frame: [heap:frame header][file:16k-32K] results in APR_EBADF > > This smells wrong - if you split a file bucket (and there is nothing wrong > with splitting a file bucket) you should end up with two file buckets, and > destroying the first file bucket while the second file bucket still exists > shouldn’t cause the second file bucket descriptor to close. The problem is not the apr_bucket_destroy(). The file bucket setaside, calls apr_file_setaside(), in core_output on a deferred pool, and then core_output clears that pool. This invalidates all still existing file buckets using that apr_file. At least, that is my reading of what happens. > Regards, > Graham > — >
Re: core_output, files and setaside
On 04 May 2016, at 10:45 AM, Stefan Eissing wrote: > I have been wrong before...but... > > mod_http2 needs to send out a file response: > 1. it starts with the response body brigade: [file:0-len][eos] > 2. it sends the first 16K frame by splitting the file bucket: > -> passing to core output: [heap:frame header][file:0-16k] > -> remaining body: [file:16K-len][eos] > 3. core_output decides to setaside: > -> setaside (deferred pool): [heap:frame header][file:0-16k] > -> remaining body: [file:16K-len][eos] > 4. core_output sends and, sometimes, clears the deferred pool > -> which closes the file descriptor > 5. next 16K frame: [heap:frame header][file:16k-32K] results in APR_EBADF This smells wrong - if you split a file bucket (and there is nothing wrong with splitting a file bucket) you should end up with two file buckets, and destroying the first file bucket while the second file bucket still exists shouldn’t cause the second file bucket descriptor to close. Regards, Graham —
core_output, files and setaside
I have been wrong before...but... mod_http2 needs to send out a file response: 1. it starts with the response body brigade: [file:0-len][eos] 2. it sends the first 16K frame by splitting the file bucket: -> passing to core output: [heap:frame header][file:0-16k] -> remaining body: [file:16K-len][eos] 3. core_output decides to setaside: -> setaside (deferred pool): [heap:frame header][file:0-16k] -> remaining body: [file:16K-len][eos] 4. core_output sends and, sometimes, clears the deferred pool -> which closes the file descriptor 5. next 16K frame: [heap:frame header][file:16k-32K] results in APR_EBADF What is different to HTTP/1? a) http/1 sends the body in one pass when it can. file buckets are never split intentionally b) http/1 sends with a request set in filter->r, so the filter brigade saving uses not a new, "deferred" pool, but the request pool. That means that the file bucket setaside becomes a NOP b) is the thing mod_http2 can currently not emulate. There is no request_rec on the master connection, and yet the buckets belong to a http2 stream whose memory pool is under control. 2.4.x seems to have similar handling in its output. What to do? I. Not sending file buckets is safe, but http: performance will suffer severely. II. Sending a new kind of "stream file" bucket that knows how to handle setaside better, would make it safe. However the sendfile() code in core will not trigger on those. III. setaside the file to conn_rec->pool and apr_file_close() explicitly at end of stream. will work, but wastes a small amount of conn_rec->pool for each file. I will go ahead and try III, but if you have another idea, I'm all ears. -Stefan
Re: Detecting client aborts and stream resets
> Am 03.05.2016 um 17:35 schrieb William A Rowe Jr : > > On Tue, May 3, 2016 at 9:31 AM, Michael Kaufmann > wrote: > Hi all, > > a content generator module can detect client aborts and stream resets while > it reads the request body. But how can it detect this afterwards, while the > response is being generated? > > This is important for HTTP/2, because the client may reset a stream, and > mod_http2 needs to wait for the content generator to finish. Therefore the > content generator should stop generating the response when it is no longer > needed. > > Is there any API for this? The "conn_rec->aborted" flag exists, but which > Apache function sets this flag? conn_rec->aborted is currently not set by mod_http2 on slave connections, but should. I'll add that. > If there is no API, maybe an optional function for mod_http2 would be a > solution. > > Nope - an optional function in mod_http2 is too special case, generators > must remain protocol (socket or other transport) agnostic. > > In the case of mod_cache'd content, the generator can't quit, it is already > populating a cache, which means you'll generate an invalid cached object. > > But if you knew that the cache module wasn't collecting info, r->c->aborted > tells you if anyone is still listening, right? William is right, this is not a good idea. The ->aborted flag should serve this purpose of telling anyone interested that this connection is not longer delivering. I will make a github release soon where that is working and you can test. -Stefan
