On Wed, 21 Nov 2007, Daniel L. Rall wrote:
> On Wed, 21 Nov 2007, David Glasser wrote:
>
> > On Nov 21, 2007 5:20 PM, Daniel Rall <dlr@collab.net> wrote:
> > >
> > > On Wed, 21 Nov 2007, Daniel L. Rall wrote:
> > >
> > > > On Tue, 20 Nov 2007, Daniel L. Rall wrote:
> > > >
> > > > > On Tue, 20 Nov 2007, David Glasser wrote:
> > > > >
> > > > > > On Nov 19, 2007 4:59 PM, Dan Christian <dchristian@google.com> wrote:
> > > > > > > Argh! Not this again. openssl has similar problems (under serf).
> > > > > > >
> > > > > > > On Nov 19, 2007 1:46 PM, David Glasser <glasser@davidglasser.net> wrote:
> > > > > > > > Apparently SQLite requires you to compile it with -DSQLITE_THREADSAFE
> > > > > > > > to be at all threadsafe. (There's a way to check at runtime
> > > > > > > > (sqlite3_threadsafe()) if this is so, but that's an experimental API
> > > > > > > > that may vanish.)
> > > > ...
> > > > > > > > Should we add something to INSTALL telling people they must build
> > > > > > > > against a threadsafe SQLite if they are going to be using a thready
> > > > > > > > server?
> > > > > > >
> > > > > > > YES! And configure should verify this. Otherwise, it looks like
> > > > > > > subversion is flaky.
> > > > > >
> > > > > > I don't think it can verify it unless we require 3.5.
> > > > >
> > > > > Yup. We could use sqlite3_threadsafe() if we can safely detect it runtime
> > > > > for dynamic builds of Subversion (e.g. via dlopen()), or via configure checks
> > > > > for static Subversion builds. While not ideal, this would allow us to detect
> > > > > cases where newer versions of SQLite which do provide sqlite3_threadsafe()
> > > > > were not compiled with -DSQLITE_THREADSAFE=1.
> > > >
> > > > Attached is a (mostly untested) sketch of what I'm suggesting.
> > >
> > > Here's an updated version of this patch which uses apr_dso.h.
> >
> > I'm not really qualified to review most of the DSO and autoconf
> > details, but it sure would nice to only have to run these checks once
> > per process. I guess you just use a static sqlite_initialized boolean
> > for this; hopefully that's threadsafe :)
>
> Definitely. (Hadn't really gotten the patch to that point of polish yet.)
> Thanks for the feedback; that's exactly what I was hoping for.
I committed a revised version to trunk in r27979 based on feedback from you
and Philip.
Does the initialization in svn_fs__sqlite_open() look okay, or should I move
it outside of the scope of that function?
- application/pgp-signature attachment: stored
Received on Thu Nov 22 09:32:17 2007