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

[cboos]

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

@@ -182 +182 @@
                 else:
                     del self._active[tid]
                     if cnx not in self._dormant:
-                        cnx.rollback()
-                        self._dormant.append(cnx)
-                        self._available.notify()
+                        del cnx
         finally:
             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.

[cmlenz]

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.

[cmlenz]

Don't reuse SQLite connections across different threads

[cboos]

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

[cmlenz]

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).

[cboos]

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.

[anonymous]

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

[jberry@…]

Previous comment was by me.

[gk5885@…]

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

[anonymous]

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.

[cboos]

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?

[gk5885@…]

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...)

[jberry@…]

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

[gk5885@…]

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...

[cmlenz]

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).

[gk5885@…]

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.

[jberry@…]

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.

[cboos]

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.

[jberry@…]

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