Bug 52528

Summary: apr_dbd_get_driver doesn't clean up nice when failing to load a module
Product: APR Reporter: Daniel Gruno <humbedooh>
Component: APR-utilAssignee: Apache Portable Runtime bugs mailinglist <bugs>
Status: NEW ---    
Severity: normal CC: bojan
Priority: P2    
Version: HEAD   
Target Milestone: ---   
Hardware: PC   
OS: All   
Attachments: Patch to close the memory leak

Description Daniel Gruno 2012-01-25 18:52:44 UTC
Whenever I use apr_dbd_get_driver to get a driver, it will clean up nicely if the driver exists, but leak a few kb every time it fails, potentially resulting in MB upon MB of trash piling up in the memory banks.

I've tested with the following snippet that would simulate an apache httpd module starting up:

//--------------------------------------------
static void register_hooks(apr_pool_t *pool) {
    int x;
    int rc;
    apr_pool_t *POOL;
	apr_dbd_driver_t* driver;
	
    module_init(pool);
    ap_hook_handler(plua_handler, NULL, NULL, APR_HOOK_LAST);
    
    for (x=0;x<3000;x++) {
        POOL = 0;
		driver = 0;
        apr_pool_create(&POOL, pool);
        apr_dbd_init(POOL);
        rc = apr_dbd_get_driver(POOL, "nonexistentdriver", &driver);
        apr_pool_clear(POOL);
        apr_pool_destroy(POOL);
    }
    
}
//--------------------------------------------

If I replace the "nonexistentdriver" element with, per se, "odbc" or "sqlite3", it will load the driver and clean up nicely with not a drop to spare, but when I use a driver name that it cannot load, it will continue to use new memory, in this case upwards of 30-40MB for those 3,000 attempts in the above snippet.

Is this a bug, and if not, how do I resolve it, so it won't gobble up all my memory?
Comment 1 Bojan Smojver 2012-01-29 07:44:38 UTC
It's not surprising, to be honest. We use a global pool for DSOs and there is some memory allocation from that pool that is done every time a DSO is being loaded. After DSO has been picked up once successfully, it will be picked up again from a hash (ergo, no allocation), but if it was not loaded successfully, the next attempt may allocate more bytes from global dso pool and so on.
Comment 2 Bojan Smojver 2012-02-03 03:04:54 UTC
Created attachment 28256 [details]
Patch to close the memory leak

Does this patch make any difference?
Comment 3 Bojan Smojver 2012-02-03 05:29:13 UTC
Comment on attachment 28256 [details]
Patch to close the memory leak

Doesn't help.
Comment 4 Bojan Smojver 2012-02-05 07:30:09 UTC
Can I ask here, why do you need to pass strings that are definitely not DB driver names into this function?
Comment 5 Stefan Fritsch 2012-08-05 21:48:53 UTC
I think apr_dbd_get_driver() should not ignore the passed in pool but pass it to apu_dso_load(). The latter will only use it for temporary data, anyway, and load the dso with the global pool.

apr_dso_load() (apr_, not apu_) will still allocate the size of a apr_dso_handle_t on failure, but that's much less than what apu_dso_load() will allocate for pathnames, etc.