possible apr_dbd_get_driver dso issue

["Brian J. France" <li...@firehawksystems.com>]

I think there is a issue with the dso code for apr_dbd_get_driver.

I was writing a milter using the APR and while reviewing the code and  
the APR dbd code I noticed that I would have big problems if I built  
the drivers as dso's.

The problem is because I use a global pool to call apr_dbd_init, but  
then call apr_dbd_get_driver using a child pool which is cleaned up  
after a request. apr_dbd_get_driver will load a dso driver using the  
child pool (calling init on the driver), but the memory will get  
cleaned up when the pool get cleaned up at the end of the request.   
This is bad because the driver will stay in the drivers hash, but  
with bogus memory.

I think it might be better to turn this:

   static apr_hash_t *drivers = NULL;

into this:

   typedef struct apr_dbd_drivers_t {
       apr_pool_t *pool;
       apr_hash_t *drivers = NULL;
   } apr_dbd_drivers_t;

and save the pool from the apr_dbd_init call to use in the  
apr_dbd_get_driver dso loading code so things match.

This is all hypothetical and I have not run into a problem yet as I  
am building things statically.

Brian

Re: possible apr_dbd_get_driver dso issue

[Joe Orton <jo...@redhat.com>]

On Thu, Mar 22, 2007 at 12:09:51PM -0400, Brian J. France wrote:
> I think there is a issue with the dso code for apr_dbd_get_driver.
> 
> I was writing a milter using the APR and while reviewing the code and  
> the APR dbd code I noticed that I would have big problems if I built  
> the drivers as dso's.
> 
> The problem is because I use a global pool to call apr_dbd_init, but  
> then call apr_dbd_get_driver using a child pool which is cleaned up  
> after a request. apr_dbd_get_driver will load a dso driver using the  
> child pool (calling init on the driver), but the memory will get  
> cleaned up when the pool get cleaned up at the end of the request.   
> This is bad because the driver will stay in the drivers hash, but  
> with bogus memory.

Ah... I thought about this before but forgot to follow through.  Thanks 
for the report.

It would be sufficient IMO to make it an API requirement that the pool 
passed to apr_dbd_get_driver is the same as the pool passed to 
apr_dbd_init().  But it looks like existing callers don't do that, so 
it's easy to just ignore the passed-in pool and DTRT:

http://svn.apache.org/viewvc?view=rev&revision=521327

It would probably be good to have apr_dbd_init() create a subpool and 
for apr_dbd_term() to destroy that subpool but that would be less 
pseudo-thread-safe than the current code....

joe