On 4/27/20 2:29 PM, Yann Ylavic wrote:
> On Mon, Apr 27, 2020 at 11:11 AM Joe Orton <jor...@redhat.com> wrote:
>>
>> On Sun, Apr 26, 2020 at 05:26:02PM +0200, Yann Ylavic wrote:
>>> For FILE buckets, the behaviour of apr_bucket_setaside() is to take
>>> *full* ownership of the underlying apr_file, that is: allocate/move
>>> the file/cleanup to the new pool AND set the old file's fd to -1 (see
>>> apr_file_setaside, [1]).
>>>
>>> This can be problematic in an output filters chain whenever a FILE
>>> bucket gets splitted. Suppose that an FTYPE_RESOURCE output filter
>>> splits a FILE bucket in two and sends the first one through the chain,
>>> and then due to network congestion the core output filter needs to set
>>> the FILE bucket aside. It happens that the second FILE bucket becomes
>>> a dangling bucket (with fd == -1).
>>
>> Hang on, I don't follow this, how does data->fd end up as -1 in this
>> case? The buckets split from the original FILE bucket have a common
>> ->data since it is a refcounted bucket type. If any of them are
>> setaside the common ->data->fd is replaced with a new apr_file_t
>> allocated from a new (longer-lived) pool. No?
>
> The call chain is:
> apr_bucket_setaside(bucket, newpool)
> => file_bucket_setaside(bucket, newpool)
> => apr_file_setaside(&newfd, bucket->data->fd, newpool)
> => copy bucket->data->fd to newfd, move cleanup to newpool
> => bucket->data->fd->filedes = -1
> => bucket->data->fd = newfd
But isn't bucket->data->fd the same for the old and the new one?
So after bucket->data->fd = newfd bucket->data->fd(aka newfd)->filedes will
have the correct value
for all buckets?
Regards
Rüdiger
