Re: What is the lifetime of apr_dbd_get_entry(...); result?

[Jan Kaluža <jk...@redhat.com>]

On 02/04/2015 02:53 PM, Jan Kaluža wrote:
> Hi,

I'm CCing httpd-dev list too, because this question has not been 
answered on APR list yet and since it causes mod_authn_dbd to stop 
working randomly with pgsql, I think it could be interesting even for 
httpd developers.

I'm also attaching example patch which fixes this issue on httpd side, 
but I think we should consider fixing it in the APR-util.

> httpd's mod_authn_dbd module currently seems to expect that lifetime of
> apr_dbd_get_entry(...) result is the same as of the pool used in
> apr_dbd_get_row(...) function.
>
> But this is not a true for at least pgsql backend. Its get_entry
> function returns "return PQgetvalue(row->res->res, row->n, n);".
> Documentation for PQgetvalue says following:
>
> "One must explicitly copy the data into other storage if it is to be
> used past the lifetime of the PGresult structure itself."
>
> res->res is also freed in dbd_pgsql_get_row(...).
>
> Is that correct behaviour and httpd's mod_authn_dbd module should strdup
> the result, or is it apr_dbd, which should return result with expected
> lifetime?
>
> Regards,
> Jan Kaluza

Regards,
Jan Kaluza

Re: What is the lifetime of apr_dbd_get_entry(...); result?

[Jan Kaluža <jk...@redhat.com>]

On 03/02/2015 11:24 AM, Yann Ylavic wrote:
> I meant to reply to all...
 >
> On Mon, Mar 2, 2015 at 11:23 AM, Yann Ylavic <yl...@gmail.com> wrote:
>> On Mon, Mar 2, 2015 at 8:22 AM, Jan Kaluža <jk...@redhat.com> wrote:
>>> On 02/04/2015 02:53 PM, Jan Kaluža wrote:
>>>
>>>> httpd's mod_authn_dbd module currently seems to expect that lifetime of
>>>> apr_dbd_get_entry(...) result is the same as of the pool used in
>>>> apr_dbd_get_row(...) function.
>>
>> It seems that the result's lifetime is really the one of the row, but
>> mod_authn_dbd keep reading rows (sequentially) to cleanup the results
>> (before exiting the loop, probably for connection keepalive purpose).
>>
>>>>
>>>> But this is not a true for at least pgsql backend. Its get_entry
>>>> function returns "return PQgetvalue(row->res->res, row->n, n);".
>>>> Documentation for PQgetvalue says following:
>>>>
>>>> "One must explicitly copy the data into other storage if it is to be
>>>> used past the lifetime of the PGresult structure itself."
>>>>
>>>> res->res is also freed in dbd_pgsql_get_row(...).
>>
>> Only the results out-of-range are freed (wrt asked rownum), but not
>> the current one(s).
>>
>>>>
>>>> Is that correct behaviour and httpd's mod_authn_dbd module should strdup
>>>> the result, or is it apr_dbd, which should return result with expected
>>>> lifetime?
>>
>> I think the fix is to be mod_authn_dbd only, APR looks correct here,
>> and so is your patch.

Thanks for answer, I've committed it in r1663647.

>> Regards,
>> Yann.

Regards,
Jan Kaluza

Re: What is the lifetime of apr_dbd_get_entry(...); result?

[Yann Ylavic <yl...@gmail.com>]

I meant to reply to all...

On Mon, Mar 2, 2015 at 11:23 AM, Yann Ylavic <yl...@gmail.com> wrote:
> On Mon, Mar 2, 2015 at 8:22 AM, Jan Kaluža <jk...@redhat.com> wrote:
>> On 02/04/2015 02:53 PM, Jan Kaluža wrote:
>>
>>> httpd's mod_authn_dbd module currently seems to expect that lifetime of
>>> apr_dbd_get_entry(...) result is the same as of the pool used in
>>> apr_dbd_get_row(...) function.
>
> It seems that the result's lifetime is really the one of the row, but
> mod_authn_dbd keep reading rows (sequentially) to cleanup the results
> (before exiting the loop, probably for connection keepalive purpose).
>
>>>
>>> But this is not a true for at least pgsql backend. Its get_entry
>>> function returns "return PQgetvalue(row->res->res, row->n, n);".
>>> Documentation for PQgetvalue says following:
>>>
>>> "One must explicitly copy the data into other storage if it is to be
>>> used past the lifetime of the PGresult structure itself."
>>>
>>> res->res is also freed in dbd_pgsql_get_row(...).
>
> Only the results out-of-range are freed (wrt asked rownum), but not
> the current one(s).
>
>>>
>>> Is that correct behaviour and httpd's mod_authn_dbd module should strdup
>>> the result, or is it apr_dbd, which should return result with expected
>>> lifetime?
>
> I think the fix is to be mod_authn_dbd only, APR looks correct here,
> and so is your patch.
>
> Regards,
> Yann.

Re: What is the lifetime of apr_dbd_get_entry(...); result?

[Yann Ylavic <yl...@gmail.com>]

I meant to reply to all...

On Mon, Mar 2, 2015 at 11:23 AM, Yann Ylavic <yl...@gmail.com> wrote:
> On Mon, Mar 2, 2015 at 8:22 AM, Jan Kaluža <jk...@redhat.com> wrote:
>> On 02/04/2015 02:53 PM, Jan Kaluža wrote:
>>
>>> httpd's mod_authn_dbd module currently seems to expect that lifetime of
>>> apr_dbd_get_entry(...) result is the same as of the pool used in
>>> apr_dbd_get_row(...) function.
>
> It seems that the result's lifetime is really the one of the row, but
> mod_authn_dbd keep reading rows (sequentially) to cleanup the results
> (before exiting the loop, probably for connection keepalive purpose).
>
>>>
>>> But this is not a true for at least pgsql backend. Its get_entry
>>> function returns "return PQgetvalue(row->res->res, row->n, n);".
>>> Documentation for PQgetvalue says following:
>>>
>>> "One must explicitly copy the data into other storage if it is to be
>>> used past the lifetime of the PGresult structure itself."
>>>
>>> res->res is also freed in dbd_pgsql_get_row(...).
>
> Only the results out-of-range are freed (wrt asked rownum), but not
> the current one(s).
>
>>>
>>> Is that correct behaviour and httpd's mod_authn_dbd module should strdup
>>> the result, or is it apr_dbd, which should return result with expected
>>> lifetime?
>
> I think the fix is to be mod_authn_dbd only, APR looks correct here,
> and so is your patch.
>
> Regards,
> Yann.