Edgewall Software
Modify

Opened 14 years ago

Closed 13 years ago

Last modified 13 years ago

#2196 closed defect (fixed)

Trac can't use the thread-safe version of SQLite on Linux

Reported by: Christian Boos Owned by: Jonas Borgström
Priority: normal Milestone: 0.9
Component: general Version: devel
Severity: normal Keywords: sqlite3 threadsafe
Cc: jberry@…, gk5885@… Branch:
Release Notes:
API Changes:

Description

If SQLite has been configured with the --enable-threadsafe switch, the resulting binary won't work with Trac:

Oops...

Trac detected an internal error:

library routine called out of sequence
Traceback (most recent call last):
  File "/opt/trac/stable-bct-trac/lib/python2.3/site-packages/trac/web/standalone.py", line 235, in _do_trac_req
    dispatch_request(path_info, req, env)
...
  File "/usr/lib/python2.3/site-packages/sqlite/main.py", line 244, in execute
    self.rs = self.con.db.execute(SQL)
ProgrammingError: library routine called out of sequence

This is because the --enable-threadsafe flag will enable, at the C level, a check for thread consistency. That check enforces that each operation involving a sqlite3* object will be performed from within the same thread that created that object, or a SQLITE_MISUSE error will be returned.

This is the same kind of check that PySQLite does when the check_same_thread=True flag is given to the Connection constructor.

Currently in Trac, we explicitely disable that check at the PySQLite level, but there's no way to disable it on the SQLite binary if it has been compiled with --enable-threadsafe.

The workaround is to compile SQLite without that flag, which, fortunately for us, is the default on Linux.

But it's not intuitive, as one would expect to turn thread safety on for a library which will be used in a multi-threaded context.

The real question is: can we assume that using SQLite that way is always safe? For now, it seems to be the case, but I guess there were reasons for having those checks in the first place.

In #1811, cmlenz says that it's OK to do what we do, but from the SQLite FAQ which is cited there, I read:

It is never safe to use the same sqlite3 structure pointer in two or more threads.

In the case of SQLite, it would be cleaner to not reuse a connection once the thread which created it terminates. The database pool idea could be kept, as it always allocate a connection to the same thread. However, the connection should be destroyed after its last use, instead of being put back in the pool. With this change, it is then possible to use the thread safe version of the SQLite library:

  • trac/db.py

     
    182182                else:
    183183                    del self._active[tid]
    184184                    if cnx not in self._dormant:
    185                         cnx.rollback()
    186                         self._dormant.append(cnx)
    187                         self._available.notify()
     185                        del cnx
    188186        finally:
    189187            self._available.release()

(tested with sqlite-3.2.7 and pysqlite-2.0.4+#ps126, on Linux).

Another approach would be to not use the database connection pool at all, and attach a new db connection to the request object (this approach was already suggested in #1721). I'm pretty sure this won't affect the performance, quite the contrary, but I haven't tested that yet.

Attachments (1)

sqlite_threading.diff (1.4 KB ) - added by Christopher Lenz 14 years ago.
Don't reuse SQLite connections across different threads

Download all attachments as: .zip

Change History (19)

comment:1 by Christopher Lenz, 14 years ago

Remember that other DB systems do not have this restriction of only being able to use a connection on the thread that created it. I'll attach an alternative patch that disabling the cross-thread pooling only for SQLite.

by Christopher Lenz, 14 years ago

Attachment: sqlite_threading.diff added

Don't reuse SQLite connections across different threads

comment:2 by Christian Boos, 14 years ago

Yes, the patch inlined in the description was just for illustration, yours is much better.

However, the db.py code becomes quite complex, especially w.r.t. inheritance and wrapping. That complexity also leads to some inefficiency, like wrapping the pysqlite2.dbapi2.Cursor to make it iterable (it already is).

I would like to eventually make a refactoring there, with a new trac/db/ subfolder:

  • api.py would provide init_db and get_cnx_pool and would take care of loading the actual database backend
  • pysqlite.py backend, with a dummy pool implementation, no need to wrap the Connection in a PoolableConnection class
  • pysqlite2.py backend, same as above for PySQLite2 + provide a cursor with unwrapped iterator, etc.
  • postgres.py backend with the PostgreSQL support
  • … this leaves the door open to more backends, without complexifying further the db.py file

comment:3 by Christopher Lenz, 14 years ago

Milestone: 0.9

I agree with this, if we actually decide to continue rolling our own DB layer, as opposed to adopting something like SQLObject. For now, that patch is just a quick fix for the issue. I think we should check this in for 0.9.

Note that the SQLite connection wrapper doesn't actually wrap the cursor in IterableCursor, because it implements cursor() itself to just return the raw cursor (or the PyFormatCursor).

comment:4 by Christian Boos, 14 years ago

Resolution: fixed
Status: newclosed
Summary: Trac can't use the thread-safe version of sqliteTrac can't use the thread-safe version of SQLite on Linux

The attachment:sqlite_threading.diff patch was applied as part of r2355.

Note that the present issue was actually Linux/Unix specific, therefore the change in the Summary.

comment:5 by anonymous, 14 years ago

Resolution: fixed
Status: closedreopened

With trac 0.9.4, Apache 2.2, mod_python 3.1.4, pysqlite 2.0.7, sqlite 3.3.4, Mac OS X 10.4.5, I'm still getting this error, or at least a similar one:

[Mon Feb 27 14:52:49 2006] [error] [client 24.20.133.214] PythonHandler trac.web.modpython_frontend: File "/opt/local/lib/python2.4/site-packages/mod_python/apache.py", line 299, in HandlerDispatch\n result = object(req) [Mon Feb 27 14:52:49 2006] [error] [client 24.20.133.214] PythonHandler trac.web.modpython_frontend: File "/opt/local/lib/python2.4/site-packages/trac/web/modpython_frontend.py", line 199, in handler\n env = get_environment(mpr, project_opts) [Mon Feb 27 14:52:49 2006] [error] [client 24.20.133.214] PythonHandler trac.web.modpython_frontend: File "/opt/local/lib/python2.4/site-packages/trac/web/main.py", line 335, in get_environment\n return _open_environment(env_path, threaded) [Mon Feb 27 14:52:49 2006] [error] [client 24.20.133.214] PythonHandler trac.web.modpython_frontend: File "/opt/local/lib/python2.4/site-packages/trac/web/main.py", line 51, in _open_environment\n env_cache[env_path] = open_environment(env_path) [Mon Feb 27 14:52:49 2006] [error] [client 24.20.133.214] PythonHandler trac.web.modpython_frontend: File "/opt/local/lib/python2.4/site-packages/trac/env.py", line 375, in open_environment\n if env.needs_upgrade(): [Mon Feb 27 14:52:49 2006] [error] [client 24.20.133.214] PythonHandler trac.web.modpython_frontend: File "/opt/local/lib/python2.4/site-packages/trac/env.py", line 279, in needs_upgrade\n db = self.get_db_cnx() [Mon Feb 27 14:52:49 2006] [error] [client 24.20.133.214] PythonHandler trac.web.modpython_frontend: File "/opt/local/lib/python2.4/site-packages/trac/env.py", line 137, in get_db_cnx\n return self.cnx_pool.get_cnx() [Mon Feb 27 14:52:49 2006] [error] [client 24.20.133.214] PythonHandler trac.web.modpython_frontend: File "/opt/local/lib/python2.4/site-packages/trac/db.py", line 157, in get_cnx\n cnx = self._cnx_class(self._args) [Mon Feb 27 14:52:49 2006] [error] [client 24.20.133.214] PythonHandler trac.web.modpython_frontend: File "/opt/local/lib/python2.4/site-packages/trac/db.py", line 280, in init\n timeout=timeout) [Mon Feb 27 14:52:49 2006] [error] [client 24.20.133.214] PythonHandler trac.web.modpython_frontend: ProgrammingError: library routine called out of sequence

comment:6 by jberry@…, 14 years ago

Previous comment was by me.

comment:7 by anonymous, 14 years ago

Cc: jberry@… added

comment:8 by gk5885@…, 14 years ago

I can confirm the reproducability of this error under the same environment.

comment:9 by anonymous, 14 years ago

It is not an issue with CGI/FastCGI backend and with Python 2.3. I'm also under Unix/Linux Debian and have no issue with the SQLite Database, and I'm using the —enable-threadsafe with the compiled version of SQlite 3.3.4.

I'm happy the previous fix was applied because that could have been an issue.

comment:10 by Christian Boos, 14 years ago

I can't reproduce the issue with sqlite-3.3.4 and pysqlite-2.1.3 (on Linux, I don't have a Mac).

jberry,gk5885: Are you sure that you've configured sqlite-3.3.4 with the --enable-threadsafe switch?

comment:11 by gk5885@…, 14 years ago

yes, definitely. "—enable-threadsafe —disable-tcl" are the ones specified. (for both of us I would imagine because it comes from darwinports) my apache error log is virtually identical to that jberry's. is there any more information anywhere that could help us figure this one out? (I am very out of my realm with python…)

comment:12 by jberry@…, 14 years ago

Yes, as gk5885 says, sqlite3 built by darwinports is built with —enable-threadsafe. Here's the configure input/output to illustrate. Note, however, the following two lines in particular:

checking whether to allow connections to be shared across threads... no
checking whether threads can override each others locks... no

I don't know if those point to an issue or not…

-jdb

cd "/opt/local/var/db/dports/build/_Users_jberry_darwinports_dports_databases_sqlite3/work/sqlite-3.3.4" && CC=/usr/bin/gcc-4.0 CPP=/usr/bin/cpp-4.0 CXX=/usr/bin/g++-4.0 ./configure --prefix=/opt/local --enable-threadsafe --disable-tcl'
checking build system type... powerpc-apple-darwin8.5.0
checking host system type... powerpc-apple-darwin8.5.0
checking for gcc... /usr/bin/gcc-4.0
checking for C compiler default output file name... a.out
checking whether the C compiler works... yes
checking whether we are cross compiling... no
checking for suffix of executables... 
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether /usr/bin/gcc-4.0 accepts -g... yes
checking for /usr/bin/gcc-4.0 option to accept ANSI C... none needed
checking for a sed that does not truncate output... /usr/bin/sed
checking for egrep... grep -E
checking for ld used by /usr/bin/gcc-4.0... /usr/bin/ld
checking if the linker (/usr/bin/ld) is GNU ld... no
checking for /usr/bin/ld option to reload object files... -r
checking for BSD-compatible nm... /usr/bin/nm -p
checking whether ln -s works... yes
checking how to recognise dependent libraries... pass_all
checking how to run the C preprocessor... /usr/bin/cpp-4.0
checking for ANSI C header files... yes
checking for sys/types.h... yes
checking for sys/stat.h... yes
checking for stdlib.h... yes
checking for string.h... yes
checking for memory.h... yes
checking for strings.h... yes
checking for inttypes.h... yes
checking for stdint.h... yes
checking for unistd.h... yes
checking dlfcn.h usability... yes
checking dlfcn.h presence... yes
checking for dlfcn.h... yes
checking whether we are using the GNU C++ compiler... yes
checking whether /usr/bin/g++-4.0 accepts -g... yes
checking how to run the C++ preprocessor... /usr/bin/g++-4.0 -E
checking for g77... no
checking for f77... no
checking for xlf... no
checking for frt... no
checking for pgf77... no
checking for fort77... no
checking for fl32... no
checking for af77... no
checking for f90... no
checking for xlf90... no
checking for pgf90... no
checking for epcf90... no
checking for f95... no
checking for fort... no
checking for xlf95... no
checking for ifc... no
checking for efc... no
checking for pgf95... no
checking for lf95... no
checking for gfortran... no
checking whether we are using the GNU Fortran 77 compiler... no
checking whether  accepts -g... no
checking the maximum length of command line arguments... 65536
checking command to parse /usr/bin/nm -p output from /usr/bin/gcc-4.0 object... ok
checking for objdir... .libs
checking for ar... ar
checking for ranlib... ranlib
checking for strip... strip
checking if /usr/bin/gcc-4.0 static flag  works... yes
checking if /usr/bin/gcc-4.0 supports -fno-rtti -fno-exceptions... no
checking for /usr/bin/gcc-4.0 option to produce PIC... -fno-common
checking if /usr/bin/gcc-4.0 PIC flag -fno-common works... yes
checking if /usr/bin/gcc-4.0 supports -c -o file.o... yes
checking whether the /usr/bin/gcc-4.0 linker (/usr/bin/ld) supports shared libraries... yes
checking dynamic linker characteristics... darwin8.5.0 dyld
checking how to hardcode library paths into programs... immediate
checking whether stripping libraries is possible... yes
checking if libtool supports shared libraries... yes
checking whether to build shared libraries... yes
checking whether to build static libraries... yes
configure: creating libtool
appending configuration tag "CXX" to libtool
checking for ld used by /usr/bin/g++-4.0... /usr/bin/ld
checking if the linker (/usr/bin/ld) is GNU ld... no
checking whether the /usr/bin/g++-4.0 linker (/usr/bin/ld) supports shared libraries... yes
checking for /usr/bin/g++-4.0 option to produce PIC... -fno-common
checking if /usr/bin/g++-4.0 PIC flag -fno-common works... yes
checking if /usr/bin/g++-4.0 supports -c -o file.o... yes
checking whether the /usr/bin/g++-4.0 linker (/usr/bin/ld) supports shared libraries... yes
checking dynamic linker characteristics... darwin8.5.0 dyld
checking how to hardcode library paths into programs... immediate
checking whether stripping libraries is possible... yes
appending configuration tag "F77" to libtool
checking for a BSD-compatible install... /usr/bin/install -c
checking for gawk... no
checking for mawk... no
checking for nawk... no
checking for awk... awk
Version set to 3.3
Release set to 3.3.4
Version number set to 3003004
checking for gcc... (cached) /usr/bin/gcc-4.0
checking whether we are using the GNU C compiler... (cached) yes
checking whether /usr/bin/gcc-4.0 accepts -g... (cached) yes
checking for /usr/bin/gcc-4.0 option to accept ANSI C... (cached) none needed
checking switches for the host compiler... -g -O2
checking target compiler... /usr/bin/gcc-4.0
checking switches on the target compiler... -g -O2
checking target linker... /usr/bin/gcc-4.0
checking switches on the target compiler... checking for ranlib... (cached) ranlib
-g -O2
checking if host and target compilers are the same... yes
checking whether to support threadsafe operation... yes
checking for pthread_create in -lpthread... yes
checking whether to allow connections to be shared across threads... no
checking whether threads can override each others locks... no
checking whether to support shared library linked as release mode or not... no
checking whether to use an in-ram database for temporary tables... no
checking if executables have the .exe suffix... unknown
checking for library containing tgetent... -lncurses
checking for readline in -lreadline... yes
checking for library containing fdatasync... no
checking readline header files... not specified: still searching...
checking readline.h usability... no
checking readline.h presence... no
checking for readline.h... no
checking for /usr/include/readline.h... no
checking for /usr/include/readline/readline.h... yes
checking for usleep... yes
checking for fdatasync... no
configure: creating ./config.status
config.status: creating Makefile
config.status: creating sqlite3.pc

comment:13 by gk5885@…, 14 years ago

Cc: gk5885@… added

i have a development… it turns out that if i use tracd on the same install, everything works. so, my first thought is that the problem has to do with mod_python. the other thing i've been looking at is the fact that there are two installs of sqlite3 on os x 10.4. there's the latest 3.3.4 from darwinports and the 3.1.3 version that comes with 10.4. is it possible for mod_python and tracd to be using different versions? is there any way to find out which is being used when the trac scripts are run by mod_python? i tested for path issues and couldn't find anything…

comment:14 by Christopher Lenz, 14 years ago

I guess tracd is using the sqlite3 version installed by DP, because it would be found first on your PATH. Apache/mod_python know nothing about your PATH though, so it uses the version that came with the OS (which I never had any problems with BTW).

comment:15 by gk5885@…, 14 years ago

well, it does seem to be an issue with mod_python, but i cannot figure out what it is… i've found more ways to break that thing than anyone could imagine. anyway, the same install also works with fastcgi so i'm pretty confident that it's not a trac problem. i'm fine with closing the ticket and assuming there's a configuration/build tweak for os x that i just haven't found yet.

comment:16 by jberry@…, 14 years ago

I am not comfortable in closing this bug. In my case, mod_python, apache, trac, and sqlite are all installed by darwinports, and I'm quite sure they do not use bits from the system.

I suspect that the reason things bust under mod_python, and not under tracd (which I can confirm) is that mod_python is a different environment, and perhaps allows more concurrency than does tracd.

My current workaround for this issue is to run tracd, with access through apache's mod_proxy.

comment:17 by Christian Boos, 13 years ago

Keywords: sqlite3 threadsafe added
Resolution: fixed
Status: reopenedclosed

jberry: if the problem persists for you (SQLite library routine called out of sequence, with mod_python, apache, trac, and sqlite all installed by darwinports), I'd suggest that you open a new ticket.

Leaving this issue open is a bit misleading, as Trac does support the thread-safe version of SQLite3 on Linux since 0.9.

comment:18 by jberry@…, 13 years ago

New ticket, #2969, opened to describe the problem, which persists. The new ticket was opened by another user with the same issue.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Jonas Borgström.
The resolution will be deleted. Next status will be 'reopened'.
to as closed The owner will be changed from Jonas Borgström to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.