Joe Orton wrote: > On Fri, Nov 21, 2008 at 06:32:58AM -0000, William Rowe wrote: >> Author: wrowe >> Date: Thu Nov 20 22:32:58 2008 >> New Revision: 719505 >> >> URL: http://svn.apache.org/viewvc?rev=719505&view=rev >> Log: >> Introduce DBM DSO linkage. >> >> Backports: 719504 > ... >> Modified: apr/apr-util/branches/1.3.x/dbm/apr_dbm.c >> URL: >> http://svn.apache.org/viewvc/apr/apr-util/branches/1.3.x/dbm/apr_dbm.c?rev=719505&r1=719504&r2=719505&view=diff >> ============================================================================== >> --- apr/apr-util/branches/1.3.x/dbm/apr_dbm.c (original) >> +++ apr/apr-util/branches/1.3.x/dbm/apr_dbm.c Thu Nov 20 22:32:58 2008 > ... >> +static apr_status_t dbm_open_type(apr_dbm_type_t const* * vtable, >> + const char *type, >> + apr_pool_t *pool) >> +{ > ... >> + >> + if (!drivers) >> + { >> + apr_pool_t *ppool = pool; >> + apr_pool_t *parent; >> + >> + /* Top level pool scope, need process-scope lifetime */ >> + for (parent = pool; parent; parent = apr_pool_parent_get(ppool)) >> + ppool = parent; >> + >> + /* deprecate in 2.0 - permit implicit initialization */ >> + apu_dso_init(ppool); >> + >> + drivers = apr_hash_make(ppool); >> + apr_hash_set(drivers, "sdbm", APR_HASH_KEY_STRING, >> &apr_dbm_type_sdbm); > > This surely isn't thread-safe? Two threads could enter apr_dbm_open*() > with drivers == NULL and then call apu_dso_init() on the same global > pool, which would break.
That is correct. Let's explore a little deeper, though? http://svn.apache.org/repos/asf/apr/apr-util/trunk/misc/apu_dso.c dsos' is a singleton, and apu_dso_init() may be (safely) called multiple times to ensure the dso services are available. If nobody has performed any database, sql, ldap or other access prior to thread creation and race-for-first, then you are right. It won't break, but it certainly can cause a double-load artifact due to the race. The point to the global shared pool is that the process-loaded provider is global to all players. The contract needs firming up, but "break" isn't quite the right word. Redundant, certainly, and in 2.0 we have a chance to make all these mechanics simpler. But this is no different than the current (and former) loadable dbd provider issue.
