On 9/10/21 11:04 AM, Ruediger Pluem wrote:
>
>
> On 9/10/21 8:55 AM, Joe Orton wrote:
>> On Fri, Sep 10, 2021 at 12:51:53AM -0000, yla...@apache.org wrote:
>>> Author: ylavic
>>> Date: Fri Sep 10 00:51:53 2021
>>> New Revision: 1893204
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1893204&view=rev
>>> Log:
>>> apr_file_setaside: don't blindly kill the old cleanup and file descriptor.
>>>
>>> There is no cleanup with APR_FOPEN_NOCLEANUP, so apr_pool_cleanup_kill() can
>>> go in the !(old_file->flags & APR_FOPEN_NOCLEANUP) block.
>>>
>>> The file descriptor can't be invalidated either, the file may be split in
>>> multiple buckets and setting aside one shouldn't invalidate the others.
>>
>> Interesting. So is the API claim that:
>>
>> * @remark After calling this function, old_file may not be used
>>
>> not really correct, or needs to be weakened?
>
> I think it is correct, for the reason you state below and the reason I
> mentioned,
> but I guess we probably need to review in httpd land if we still use the
> old_file
> afterwards e.g. when buckets have been split.
I think the old code with setting the file descriptor of old_file to -1, should
reveal
in a clear way if we still use the old file afterwards. Keeping the file
descriptor may
lead to subtle failures some time later that are hard to debug.
Regards
Rüdiger
