Re: eor bucket
Thanks, Yann. Just proposed these for backport. > Am 09.05.2019 um 12:04 schrieb Yann Ylavic : > > r1707362
AW: eor bucket
C2 General > -Ursprüngliche Nachricht- > Von: Yann Ylavic > Gesendet: Donnerstag, 9. Mai 2019 12:00 > An: httpd-dev > Betreff: Re: eor bucket > > On Thu, May 9, 2019 at 11:54 AM Yann Ylavic > wrote: > > > > On Thu, May 9, 2019 at 11:37 AM Stefan Eissing > > wrote: > > > > > > > Am 09.05.2019 um 11:32 schrieb Yann Ylavic : > > > > > > > > The issue is more that the hooks in eor_bucket_cleanup() will be > run > > > > multiple times, rather than a lifetime issue. > > > > > > I read it like this: > > > The cleanup is only registered on the first creation. The copy never > registers. But the bucket_destroy calls the pool destroy each time. > > > > Ah yes, apr_bucket_copy() (i.e. apr_bucket_simple_copy()) won't do the > > right thing for eor, there should be an eor_bucket_copy() to register > > the cleanup for the copy too. > > > > We should also possibly split the eor cleanup in two, one (registered > > first) to run the hooks, and the second for NULLing b->data. > > Both (but mainly the first one) would do nothing is b->data is already > > NULL. That way I think the eor can be safely copied. > > Or, use the EOR bucket code from trunk... Which is the MMAP/file pattern approach as I just noticed :-) Thanks for the pointer. Regards Rüdiger
AW: eor bucket
C2 General > -Ursprüngliche Nachricht- > Von: Yann Ylavic > Gesendet: Donnerstag, 9. Mai 2019 11:54 > An: httpd-dev > Betreff: Re: eor bucket > > On Thu, May 9, 2019 at 11:37 AM Stefan Eissing > wrote: > > > > > Am 09.05.2019 um 11:32 schrieb Yann Ylavic : > > > > > > The issue is more that the hooks in eor_bucket_cleanup() will be run > > > multiple times, rather than a lifetime issue. > > > > I read it like this: > > The cleanup is only registered on the first creation. The copy never > registers. But the bucket_destroy calls the pool destroy each time. > > Ah yes, apr_bucket_copy() (i.e. apr_bucket_simple_copy()) won't do the > right thing for eor, there should be an eor_bucket_copy() to register > the cleanup for the copy too. > > We should also possibly split the eor cleanup in two, one (registered > first) to run the hooks, and the second for NULLing b->data. > Both (but mainly the first one) would do nothing is b->data is already > NULL. That way I think the eor can be safely copied. I think we should follow a similar design pattern as we do with MMAP/file buckets which use a refcount. Regards Rüdiger
Re: eor bucket
On Thu, May 9, 2019 at 12:00 PM Yann Ylavic wrote: > > On Thu, May 9, 2019 at 11:54 AM Yann Ylavic wrote: > > > > On Thu, May 9, 2019 at 11:37 AM Stefan Eissing > > wrote: > > > > > > > Am 09.05.2019 um 11:32 schrieb Yann Ylavic : > > > > > > > > The issue is more that the hooks in eor_bucket_cleanup() will be run > > > > multiple times, rather than a lifetime issue. > > > > > > I read it like this: > > > The cleanup is only registered on the first creation. The copy never > > > registers. But the bucket_destroy calls the pool destroy each time. > > > > Ah yes, apr_bucket_copy() (i.e. apr_bucket_simple_copy()) won't do the > > right thing for eor, there should be an eor_bucket_copy() to register > > the cleanup for the copy too. > > > > We should also possibly split the eor cleanup in two, one (registered > > first) to run the hooks, and the second for NULLing b->data. > > Both (but mainly the first one) would do nothing is b->data is already > > NULL. That way I think the eor can be safely copied. > > Or, use the EOR bucket code from trunk... That is, including r1707084 and follow ups (r1707093, r1707159 and r1707362).
Re: eor bucket
On Thu, May 9, 2019 at 11:54 AM Yann Ylavic wrote: > > On Thu, May 9, 2019 at 11:37 AM Stefan Eissing > wrote: > > > > > Am 09.05.2019 um 11:32 schrieb Yann Ylavic : > > > > > > The issue is more that the hooks in eor_bucket_cleanup() will be run > > > multiple times, rather than a lifetime issue. > > > > I read it like this: > > The cleanup is only registered on the first creation. The copy never > > registers. But the bucket_destroy calls the pool destroy each time. > > Ah yes, apr_bucket_copy() (i.e. apr_bucket_simple_copy()) won't do the > right thing for eor, there should be an eor_bucket_copy() to register > the cleanup for the copy too. > > We should also possibly split the eor cleanup in two, one (registered > first) to run the hooks, and the second for NULLing b->data. > Both (but mainly the first one) would do nothing is b->data is already > NULL. That way I think the eor can be safely copied. Or, use the EOR bucket code from trunk...
Re: eor bucket
On Thu, May 9, 2019 at 11:37 AM Stefan Eissing wrote: > > > Am 09.05.2019 um 11:32 schrieb Yann Ylavic : > > > > The issue is more that the hooks in eor_bucket_cleanup() will be run > > multiple times, rather than a lifetime issue. > > I read it like this: > The cleanup is only registered on the first creation. The copy never > registers. But the bucket_destroy calls the pool destroy each time. Ah yes, apr_bucket_copy() (i.e. apr_bucket_simple_copy()) won't do the right thing for eor, there should be an eor_bucket_copy() to register the cleanup for the copy too. We should also possibly split the eor cleanup in two, one (registered first) to run the hooks, and the second for NULLing b->data. Both (but mainly the first one) would do nothing is b->data is already NULL. That way I think the eor can be safely copied.
Re: eor bucket
> Am 09.05.2019 um 11:32 schrieb Yann Ylavic : > > The issue is more that the hooks in eor_bucket_cleanup() will be run > multiple times, rather than a lifetime issue. I read it like this: The cleanup is only registered on the first creation. The copy never registers. But the bucket_destroy calls the pool destroy each time. > > On Thu, May 9, 2019 at 11:28 AM Yann Ylavic wrote: >> >> No it's not actually, nevermind. >> >> On Thu, May 9, 2019 at 11:24 AM Yann Ylavic wrote: >>> >>> Hmm, if r->pool gets destroyed by the first eor, the >>> eor_bucket_cleanup() for the copy should NULLify its b->data at the >>> same time, so it should be safe no? >>> >>> On Thu, May 9, 2019 at 11:22 AM Plüm, Rüdiger, Vodafone Group >>> wrote: >>>> >>>> I think your understanding is correct. >>>> >>>> Regards >>>> >>>> Rüdiger >>>> >>>> >>>> C2 General >>>> >>>>> -Ursprüngliche Nachricht- >>>>> Von: Stefan Eissing >>>>> Gesendet: Donnerstag, 9. Mai 2019 11:02 >>>>> An: dev@httpd.apache.org >>>>> Betreff: eor bucket >>>>> >>>>> Could someone help me to check my understanding of the eor bucket >>>>> implementation? >>>>> >>>>> If an eor bucket is ever copied, there are 2 buckets with their b->data >>>>> pointing to the request_rec. Since this is local to the bucket, >>>>> destroying these 2 will call apr_pool_destroy() twice on the pool. >>>>> Correct? >>>>> >>>>> -Stefan
Re: eor bucket
The issue is more that the hooks in eor_bucket_cleanup() will be run multiple times, rather than a lifetime issue. On Thu, May 9, 2019 at 11:28 AM Yann Ylavic wrote: > > No it's not actually, nevermind. > > On Thu, May 9, 2019 at 11:24 AM Yann Ylavic wrote: > > > > Hmm, if r->pool gets destroyed by the first eor, the > > eor_bucket_cleanup() for the copy should NULLify its b->data at the > > same time, so it should be safe no? > > > > On Thu, May 9, 2019 at 11:22 AM Plüm, Rüdiger, Vodafone Group > > wrote: > > > > > > I think your understanding is correct. > > > > > > Regards > > > > > > Rüdiger > > > > > > > > > C2 General > > > > > > > -Ursprüngliche Nachricht- > > > > Von: Stefan Eissing > > > > Gesendet: Donnerstag, 9. Mai 2019 11:02 > > > > An: dev@httpd.apache.org > > > > Betreff: eor bucket > > > > > > > > Could someone help me to check my understanding of the eor bucket > > > > implementation? > > > > > > > > If an eor bucket is ever copied, there are 2 buckets with their b->data > > > > pointing to the request_rec. Since this is local to the bucket, > > > > destroying these 2 will call apr_pool_destroy() twice on the pool. > > > > Correct? > > > > > > > > -Stefan
Re: eor bucket
> Am 09.05.2019 um 11:28 schrieb Yann Ylavic : > > No it's not actually, nevermind. Yeah, I was going back and forth like that myself. Therefore my question. ;) I am seeing in an uncomitted change a rare occasion where eor_bucket_destroy() seems to destroy a pool twice. If that is related to a bucket copy, I have not found out yet. But it's one more peculiarity to keep in mind... Thanks, guys! > On Thu, May 9, 2019 at 11:24 AM Yann Ylavic wrote: >> >> Hmm, if r->pool gets destroyed by the first eor, the >> eor_bucket_cleanup() for the copy should NULLify its b->data at the >> same time, so it should be safe no? >> >> On Thu, May 9, 2019 at 11:22 AM Plüm, Rüdiger, Vodafone Group >> wrote: >>> >>> I think your understanding is correct. >>> >>> Regards >>> >>> Rüdiger >>> >>> >>> C2 General >>> >>>> -Ursprüngliche Nachricht- >>>> Von: Stefan Eissing >>>> Gesendet: Donnerstag, 9. Mai 2019 11:02 >>>> An: dev@httpd.apache.org >>>> Betreff: eor bucket >>>> >>>> Could someone help me to check my understanding of the eor bucket >>>> implementation? >>>> >>>> If an eor bucket is ever copied, there are 2 buckets with their b->data >>>> pointing to the request_rec. Since this is local to the bucket, >>>> destroying these 2 will call apr_pool_destroy() twice on the pool. >>>> Correct? >>>> >>>> -Stefan
Re: eor bucket
No it's not actually, nevermind. On Thu, May 9, 2019 at 11:24 AM Yann Ylavic wrote: > > Hmm, if r->pool gets destroyed by the first eor, the > eor_bucket_cleanup() for the copy should NULLify its b->data at the > same time, so it should be safe no? > > On Thu, May 9, 2019 at 11:22 AM Plüm, Rüdiger, Vodafone Group > wrote: > > > > I think your understanding is correct. > > > > Regards > > > > Rüdiger > > > > > > C2 General > > > > > -Ursprüngliche Nachricht- > > > Von: Stefan Eissing > > > Gesendet: Donnerstag, 9. Mai 2019 11:02 > > > An: dev@httpd.apache.org > > > Betreff: eor bucket > > > > > > Could someone help me to check my understanding of the eor bucket > > > implementation? > > > > > > If an eor bucket is ever copied, there are 2 buckets with their b->data > > > pointing to the request_rec. Since this is local to the bucket, > > > destroying these 2 will call apr_pool_destroy() twice on the pool. > > > Correct? > > > > > > -Stefan
Re: eor bucket
Hmm, if r->pool gets destroyed by the first eor, the eor_bucket_cleanup() for the copy should NULLify its b->data at the same time, so it should be safe no? On Thu, May 9, 2019 at 11:22 AM Plüm, Rüdiger, Vodafone Group wrote: > > I think your understanding is correct. > > Regards > > Rüdiger > > > C2 General > > > -Ursprüngliche Nachricht- > > Von: Stefan Eissing > > Gesendet: Donnerstag, 9. Mai 2019 11:02 > > An: dev@httpd.apache.org > > Betreff: eor bucket > > > > Could someone help me to check my understanding of the eor bucket > > implementation? > > > > If an eor bucket is ever copied, there are 2 buckets with their b->data > > pointing to the request_rec. Since this is local to the bucket, > > destroying these 2 will call apr_pool_destroy() twice on the pool. > > Correct? > > > > -Stefan
AW: eor bucket
I think your understanding is correct. Regards Rüdiger C2 General > -Ursprüngliche Nachricht- > Von: Stefan Eissing > Gesendet: Donnerstag, 9. Mai 2019 11:02 > An: dev@httpd.apache.org > Betreff: eor bucket > > Could someone help me to check my understanding of the eor bucket > implementation? > > If an eor bucket is ever copied, there are 2 buckets with their b->data > pointing to the request_rec. Since this is local to the bucket, > destroying these 2 will call apr_pool_destroy() twice on the pool. > Correct? > > -Stefan
eor bucket
Could someone help me to check my understanding of the eor bucket implementation? If an eor bucket is ever copied, there are 2 buckets with their b->data pointing to the request_rec. Since this is local to the bucket, destroying these 2 will call apr_pool_destroy() twice on the pool. Correct? -Stefan