Bug 166319 - sqlite should be built with --enable-threadsafe
Summary: sqlite should be built with --enable-threadsafe
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: sqlite
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Paul Nasrat
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks: 177422
TreeView+ depends on / blocked
 
Reported: 2005-08-19 04:44 UTC by Nicholas Miell
Modified: 2007-11-30 22:11 UTC (History)
3 users (show)

Fixed In Version: 3.2.8-1
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-01-24 20:26:06 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
Patch sqlite.spec (927 bytes, patch)
2005-08-20 18:25 UTC, Nicholas Miell
no flags Details | Diff
Remove two-thread spinup in threading implementation test (574 bytes, patch)
2006-01-22 21:41 UTC, Bojan Smojver
no flags Details | Diff

Description Nicholas Miell 2005-08-19 04:44:12 UTC
If sqlite isn't configured with --enable-threadsafe, threaded apps which access
the same sqlite database from multiples threads will corrupt the db.

(POSIX advisory file locks are completely useless in multi-threaded apps and
--enable-threadsafe turns on the internal workarounds to deal with this cruft.)

Comment 1 Nicholas Miell 2005-08-20 18:25:40 UTC
Created attachment 117948 [details]
Patch sqlite.spec

Comment 2 Jeffrey C. Ollie 2005-10-19 18:06:18 UTC
I'd like to second Nicholas' motion.  OpenPBX is replacing the internal copy of
DB1 that it inherited from Asterisk with SQLite, but OpenPBX needs a threadsafe
version of SQLite.

Comment 3 Bojan Smojver 2006-01-21 09:47:03 UTC
Yes, please. If we could have that in FC5, it would be great.

Devel list thread:

http://www.redhat.com/archives/fedora-devel-list/2006-January/msg01028.html

Comment 4 Bojan Smojver 2006-01-22 08:25:38 UTC
BTW, can we also see a bump to 3.2.8 at the same time. It's been out since 19
Dec 2005.

Comment 5 Bojan Smojver 2006-01-22 21:41:42 UTC
Created attachment 123549 [details]
Remove two-thread spinup in threading implementation test

SQLite tests if threads can overwrite each others locks by spinning up two
threads every time the library is initialized (in fact, on the first DB file
open). Since we know how is this going to end on Fedora (bar any arch
differences I didn't test for), we should set this value to 1 and be done with
it. Saves spinning up 2 threads.

I tested this with current Rawhide on an i686.

Comment 6 Bojan Smojver 2006-01-22 21:47:23 UTC
I briefly and completely unscientifically tested threaded v. non-threaded SQLite
on by box, when running one thread of execution (i.e. the case we would care
about if thread safetly interfered with performance). The test involved
inserting 3000 records into a table. I did not observe any noticeable slowdown
with the threading implementation.

Thread safe (--enable-threadsafe):

real    1m27.286s
user    0m4.244s
sys     0m23.841s

Thread unsafe:

real    1m27.571s
user    0m4.480s
sys     0m24.334s

Basically, SQLite simply creates a pthread_mutex and acquires a lock on it
before doing things with a file. With NPTL, this should not be a huge problem.

Comment 7 Bojan Smojver 2006-01-22 22:16:58 UTC
One more thing - can you also bump sqlite2 to 2.8.17? Fixes the same database
corruption 3.2.8 does.

Comment 8 Bojan Smojver 2006-01-22 23:38:40 UTC
This bug may be somewhat related to bug #177422.

Comment 9 Alexander Larsson 2006-01-23 08:18:50 UTC
This is also important for beagle/f-spot if we switch them to sqlite3.
I'm doing the sqlite2 update to 2.8.17.

Comment 10 Bojan Smojver 2006-01-23 09:14:12 UTC
Thanks Alex.

BTW, did you get my note from bug #177422 about the actual beagle situation - it
looks like only the alpha version 3.3.1 addresses this properly (i.e. use of
connections opened in one thread by another thread). I don't have much SQLite
experience, but it appears that current stable code checks the TID and refuses
to use the connection structure if it didn't originate in that thread...

Comment 11 Bojan Smojver 2006-01-24 10:16:38 UTC
FYI, 3.3.2 beta is out.

Comment 12 Paul Nasrat 2006-01-24 20:26:06 UTC
Sorry for delay - bumped to 3.2.8 and added thread safe.

Comment 13 Bojan Smojver 2006-01-24 23:36:57 UTC
What did you think of the "save two thread spinup" patch? Too risky?

Comment 14 Paul Nasrat 2006-01-25 00:34:43 UTC
Have you tried pushing that patch upstream at all? If so please link to the
relevent mailing list discussion. The general rule in Fedora is to try and get
things upstream.

Comment 15 Bojan Smojver 2006-01-25 01:12:51 UTC
No, but I think it wouldn't get accepted even if I tried in its current form.
This is a Fedora specific thing where we know things will be certain way at
runtime. SQLite, being a cross platform thing, probably would prefer to check
what's actually going on at runtime in its default build (i.e. the binary SQLite
folks ship may not know if it's going to be on a platform that supports this or
not).

A proper upstream fix would probably be something along the lines of:

- have a configure option for this (i.e. explicitly choose certain behaviour)
- have a configure test for this (i.e. determine things at build time, rather
than runtime)

No biggie. If I find time, I may propose one of the above upstream. I just
wanted to make Fedora SQLite a tad faster by not spinning up two unnecessary
threads, which, if you use SQLite from Apache for instance, ends up being two
threads in each preforked process. I know that NPTL is very efficient and all,
but...

Comment 16 Bojan Smojver 2006-01-31 19:03:36 UTC
I contacted the author of SQLite, D. Richard Hipp, and he doesn't mind the
patches along the lines of what I discussed in commend #15. In fact, he will
patch os_unix.c file himself and he asked me to work on the patch for configure.ac.

So, there is a good chance that we'll get --enable-threads-override-locks or
something to disable two thread spinup on startup.

Comment 17 Bojan Smojver 2006-02-01 02:00:26 UTC
The SQLite CVS now has --enable-threads-override-locks, which will do what I
talked about in comment #5. Hopefully, we'll see 3.3.4 released before FC5...


Note You need to log in before you can comment on or make changes to this bug.