On 21/06/18 10:36, Rainer Jung wrote:
> Am 21.06.2018 um 10:24 schrieb jean-frederic clere:
>> On 21/06/18 10:04, Yann Ylavic wrote:
>>> On Thu, Jun 21, 2018 at 8:00 AM, jean-frederic clere
>>> <jfcl...@gmail.com> wrote:
>>>>
>>>> OK if everyone agrees I will just commit my small fix and don't
>>>> backport
>>>> the 2 other revisions.
>>>
>>> Are we looking at the same 1.6.x code?
>>> For me apr_file_buffer_set() locks the mutex unconditionally so it
>>> must unlock it likewise, I don't understand what your patch solves.
>>
>> that is what my patch is doing ;-)
>
> Please be more explicit. I agree with Yann. From current unpatched APR
> 1.6.x file_io/win32/buffer.c:
>
> APR_DECLARE(apr_status_t) apr_file_buffer_set(apr_file_t *file,
> char * buffer,
> apr_size_t bufsize)
> {
> ...
> apr_thread_mutex_lock(file->mutex);
>
> if(file->buffered) {
> ...
> if (rv != APR_SUCCESS) {
> apr_thread_mutex_unlock(file->mutex);
> return rv;
> }
> }
>
> ... (no return)
>
> apr_thread_mutex_unlock(file->mutex);
>
> return APR_SUCCESS;
> }
>
> So where's the problem? Unconditional "apr_thread_mutex_lock" and
> unconditional "apr_thread_mutex_unlock" on every path before return.
>
> Looking at your patch
> "http://people.apache.org/~jfclere/patches/patch.180618.txt"; (STATUS
> still contains a broken link, so we need to guess):
>
> Index: file_io/win32/buffer.c
> ===================================================================
> --- file_io/win32/buffer.c (revision 1833783)
> +++ file_io/win32/buffer.c (working copy)
> @@ -47,8 +47,10 @@
> */
> file->buffered = 0;
> }
> -
> - apr_thread_mutex_unlock(file->mutex);
> +
> + if (file->buffered) {
> + apr_thread_mutex_unlock(file->mutex);
> + }
>
> return APR_SUCCESS;
> }
>
>
> any call to "apr_file_buffer_set" for a file with "buffered" not set
> will end with not giving back the lock.
Correct... My problem is that file->mutex is NULL not that the file is
buffered or not...
>
> Regards,
>
> Rainer
>
--
Cheers
Jean-Frederic
