Commit ed1a090b authored by Tim Peters's avatar Tim Peters

Close a thread race in Connection.close(), diagnosed by Marius Gedminas.

DB._closeConnection() sets the connection's _db member to None now, under
protection of the lock it holds anyway.  At a deeper level, it's unclear
why _db keeps getting set and unset to begin with, but no time for that
now (and there are already XXX comments about it in the code).

Alas, I wasn't able to write a test that provoked the original bug in
finite time (it's a tiny timing hole), except via calling sleep() between
two lines that don't exist anymore.  Good enough.
parent f7b96aea
...@@ -13,7 +13,7 @@ ...@@ -13,7 +13,7 @@
############################################################################## ##############################################################################
"""Database connection support """Database connection support
$Id: Connection.py,v 1.144 2004/04/06 21:47:12 tim_one Exp $""" $Id: Connection.py,v 1.145 2004/04/08 18:12:25 tim_one Exp $"""
import logging import logging
import sys import sys
...@@ -466,7 +466,9 @@ class Connection(ExportImport, object): ...@@ -466,7 +466,9 @@ class Connection(ExportImport, object):
# Return the connection to the pool. # Return the connection to the pool.
if self._db is not None: if self._db is not None:
self._db._closeConnection(self) self._db._closeConnection(self)
self._db = None # _closeConnection() set self._db to None. However, we can't
# assert that here, because self may have been reused (by
# another thread) by the time we get back here.
def commit(self, obj, transaction): def commit(self, obj, transaction):
if obj is self: if obj is self:
......
...@@ -13,7 +13,7 @@ ...@@ -13,7 +13,7 @@
############################################################################## ##############################################################################
"""Database objects """Database objects
$Id: DB.py,v 1.72 2004/04/01 03:56:58 jeremy Exp $""" $Id: DB.py,v 1.73 2004/04/08 18:12:25 tim_one Exp $"""
import cPickle, cStringIO, sys import cPickle, cStringIO, sys
from thread import allocate_lock from thread import allocate_lock
...@@ -72,7 +72,7 @@ class DB(object): ...@@ -72,7 +72,7 @@ class DB(object):
setCacheDeactivateAfter, setCacheDeactivateAfter,
getVersionCacheDeactivateAfter, setVersionCacheDeactivateAfter getVersionCacheDeactivateAfter, setVersionCacheDeactivateAfter
""" """
klass = Connection # Class to use for connections klass = Connection # Class to use for connections
_activity_monitor = None _activity_monitor = None
...@@ -163,14 +163,21 @@ class DB(object): ...@@ -163,14 +163,21 @@ class DB(object):
return m return m
def _closeConnection(self, connection): def _closeConnection(self, connection):
"""Return a connection to the pool""" """Return a connection to the pool.
connection._db must be self on entry.
"""
self._a() self._a()
try: try:
assert connection._db is self
connection._db = None
am = self._activity_monitor am = self._activity_monitor
if am is not None: if am is not None:
am.closedConnection(connection) am.closedConnection(connection)
version=connection._version version = connection._version
pools,pooll=self._pools pools, pooll = self._pools
try: try:
pool, allocated, pool_lock = pools[version] pool, allocated, pool_lock = pools[version]
except KeyError: except KeyError:
...@@ -184,7 +191,7 @@ class DB(object): ...@@ -184,7 +191,7 @@ class DB(object):
return return
pool.append(connection) pool.append(connection)
if len(pool)==1: if len(pool) == 1:
# Pool now usable again, unlock it. # Pool now usable again, unlock it.
pool_lock.release() pool_lock.release()
finally: finally:
...@@ -708,7 +715,7 @@ class AbortVersion(ResourceManager): ...@@ -708,7 +715,7 @@ class AbortVersion(ResourceManager):
def __init__(self, db, version): def __init__(self, db, version):
super(AbortVersion, self).__init__(db) super(AbortVersion, self).__init__(db)
self._version = version self._version = version
def commit(self, ob, t): def commit(self, ob, t):
tid, oids = self._db._storage.abortVersion(self._version, t) tid, oids = self._db._storage.abortVersion(self._version, t)
self._db.invalidate(tid, dict.fromkeys(oids, 1), version=self._version) self._db.invalidate(tid, dict.fromkeys(oids, 1), version=self._version)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment