Re: sqlite threadsafe

On Thu, 22 Nov 2007, Philip Martin wrote:

> "Daniel L. Rall" <dlr@finemaltcoding.com> writes:
>
> > 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.
>
> I don't think your behaviour is common. I expect people on unusual
> platforms, AIX say, build everything in one go, and people on
> Linux/Windows install pre-built packages.
>
> Why would someone choose to upgrade a threadsafe SQLite build to a
> non-threadsafe one? If it was simple to catch we should do it, but I
> won't loose sleep if we simply ignore the problem.

When a user types 'yum install' or 'rpm -U' (or equivalent), they
often have no knowledge of exactly what they are "upgrading" to.
Typically, this situation occurs when some other package the user
wants to install or upgrade requires a newer version of a dependency
(e.g. SQLite); their package management system is likely performing
the "upgrade" for them.

I've witnessed that this as a very common situation, especially with
as widely used a dependency as SQLite.

> >> 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.
>
> Libtool, on my Linux box, creates an executable in the build tree
> linked to libraries in the build tree, but that executable doesn't
> load DSOs from the build tree. At least, that's what it did in the
> past, and unless something has changed in APR I assume it still does.
> That means that apr_dso_load/dlopen does not DTRT as you claim, even
> if it happens to work some of the time.

My understanding for *nix systems is this: DSO loading is dependent
upon the search path (e.g. as defined by /etc/ld.so.conf or
LD_LIBRARY_PATH), or absolute path to the library. If the build
location isn't in the loader's search path, and binaries weren't built
with an rpath which added the build location to the loader's search
path, then the DSO won't be loaded at runtime (or the wrong DSO will
be found and loaded from elsewhere on the system).

While I don't typically do in-tree builds of external dependencies, a
rpath (or LD_RUN_PATH) properly set at build-time would probably
handle correct DSO loading of in-tree dependencies.

IIRC, Debian libtool has some issues with rpath (e.g. it doesn't use
it), which probably account for the Subversion RA/FS DSO loading
issues you pointed out earlier in the thread.

 
> >> 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.
>
> I was thinking it's a bit like --disable-neon-version-check. I'd
> simply document that SQLite should be threadsafe and leave it up to
> the user.

*nod* I'm not a big fan of the way we do Neon version checking. I'd
rather we filtered out known broken releases, than the way we
currently disallow new releases without this flag.