On Wed, 21 Nov 2007, Philip Martin wrote:
> Daniel Rall <dlr@collab.net> writes:
>
> > +/* If possible, verify that SQLite was compiled in a thread-safe
> > + manner. The library will be unloaded when POOL is destroyed. */
> > +static svn_error_t *
> > +init_sqlite(apr_pool_t *pool)
> > +{
> > + svn_boolean_t is_threadsafe = TRUE;
> > +
> > + /* SQLite 3.5 allows verification of its thread-safety at runtime.
> > + Older versions are simply expected to have been configured with
> > + --enable-threadsafe, which compiles with -DSQLITE_THREADSAFE=1
> > + (or -DTHREADSAFE, for older versions). */
> > +#ifdef SVN_HAVE_SQLITE_THREADSAFE_PREDICATE
> > + /* sqlite3_threadsafe() was available at Subversion 'configure'-time. */
> > + is_threadsafe = sqlite3_threadsafe();
> > +
> > +#elsif APR_HAS_DSO
> > + /* Attempt to dynamically load sqlite3_threadsafe(), since the
> > + version of SQLite present at compile-time may've changed. */
> > + apr_status_t status;
> > + apr_dso_handle_t *dso = NULL;
> > + apr_dso_handle_sym_t symbol;
> > +
> > +#if DARWIN
> > + status = apr_dso_load(&dso, "libsqlite3.dylib", pool);
> > +#endif
> > + if (dso == NULL)
> > + status = apr_dso_load(&dso, "libsqlite3.so", pool);
Philip, thanks for the review!
> If I understand this correctly you are attempting to dlopen a shared
> library that is already dynamically linked to the executable. My
> first reaction is yuck!
Yes, that's right. The idea was to suppor the use case where Subversion is
dynamically linked against a pre-3.5 version of SQLite (which basically every
current OS and package management is using), then SQLite is later upgraded.
Personally, I tend to do updates like this all the time.
> That will only work if apr_dso_load uses the
> same search path as the linker, and I seem to recall that people have
> problems running the tests when RA/FS are loaded using apr_dso_load as
> the search path doesn't do what we want, it doesn't include the build
> tree for example.
On Linux and Mac OS X, apr_dso_load() wraps dlopen(), which DTRT for an
installed Subversion. I'm unsure about Windows, and also about uninstalled
builds used for testing which have dynamic linkage (would rpath come into
play here an help out?).
I did have some concern about someone building against a SQLite that's not
in the standard loader path, then getting unexpected behavior at runtime
if the loader picks up a different version of SQLite.
> I think you should avoid apr_dso_load, if configure doesn't detect
> sqlite_threadsafe then it should simply do not call it.
I considered this (and am certainly not against it), but it does fail to
take advantage of SQLite's thread-safety checks when upgrades occur and
they become available, a case which will be very common over the next couple
years. This is somewhat balanced out by the fact that the thread-safe mode
is the default in the latest SQLite CVS.
> If you think
> that is not good enough then perhaps we could require an additional
> configure option, --enable-assume-sqlite-is-threadsafe say, and only
> allow people to use an sqlite without sqlite_threadsafe if they enable
> the extra option.
Interesting suggestion. It seems like something of an unintuitive and undue
burden on package maintainers.
- application/pgp-signature attachment: stored
Received on Thu Nov 22 04:34:32 2007