Edgewall Software

Ticket #9111: t9111-create-close-outside-cs.diff

File t9111-create-close-outside-cs.diff, 5.3 KB (added by cboos, 23 months ago)

pool: when taking or returning a Connection, don't do any potentially lenghty operation with the lock held. (patch on r9929)

  • trac/db/pool.py 

    # HG changeset patch
# Parent 04aa8ff1c31795f341c2b9c571173fe7a0627bb3
pool: when taking or returning a Connection, don't do any potentially lenghty operation with the lock held.

diff --git a/trac/db/pool.py b/trac/db/pool.py
    a b class ConnectionPoolBackend(object): 
    8080        key = unicode(kwargs) 
    8181        start = time.time() 
    8282        tid = threading._get_ident() 
     83        # Get a Connection, either directly or a deferred one 
    8384        self._available.acquire() 
    8485        try: 
    8586            # First choice: Return the same cnx already used by the thread 
    … … class ConnectionPoolBackend(object): 
    9798                num = 1 
    9899            if cnx: 
    99100                self._active[(tid, key)] = (cnx, num) 
    100                 return PooledConnection(self, cnx, key, tid, log) 
    101             else: 
    102                 # if we didn't get a cnx after wait() something's fishy... 
    103                 timeout = time.time() - start 
    104                 raise TimeoutError(_('Unable to get database ' 
    105                                      'connection within %(time)d ' 
    106                                      'seconds', time=timeout)) 
    107101        finally: 
    108102            self._available.release() 
    109103 
     104        deferred = num == 1 and isinstance(cnx, tuple) 
     105        if deferred: 
     106            # Potentially lenghty operations must be done without lock held 
     107            op, cnx = cnx 
     108            if op == 'ping': 
     109                try: 
     110                    cnx.ping() 
     111                except: 
     112                    cnx = None 
     113            elif op == 'close': 
     114                cnx.close() 
     115            if op in ('close', 'create'): 
     116                cnx = connector.get_connection(**kwargs) 
     117 
     118        if cnx: 
     119            if deferred: 
     120                # replace placeholder with real Connection 
     121                self._available.acquire() 
     122                try: 
     123                    self._active[(tid, key)] = (cnx, num) 
     124                finally: 
     125                    self._available.release() 
     126            return PooledConnection(self, cnx, key, tid, log) 
     127 
     128        if deferred and op == 'ping': 
     129            # cnx couldn't be reused, clear placeholder and retry 
     130            self._available.acquire() 
     131            try: 
     132                del self._active[(tid, key)] 
     133            finally: 
     134                self._available.release() 
     135            return self.get_cnx(connector, kwargs) 
     136 
     137        # if we didn't get a cnx after wait(), something's fishy... 
     138        timeout = time.time() - start 
     139        raise TimeoutError(_("Unable to get database connection within " 
     140                             "%(time)d seconds", time=timeout)) 
     141 
    110142    def _take_cnx(self, connector, kwargs, key, tid): 
    111143        """Note: _available lock must be held when calling this method.""" 
    112         create = False 
    113144        # Second best option: Reuse a live pooled connection 
    114145        if key in self._pool_key: 
    115146            idx = self._pool_key.index(key) 
    … … class ConnectionPoolBackend(object): 
    119150            # If possible, verify that the pooled connection is 
    120151            # still available and working. 
    121152            if hasattr(cnx, 'ping'): 
    122                 try: 
    123                     cnx.ping() 
    124                 except: 
    125                     cnx = self._take_cnx(key, tid) 
     153                return ('ping', cnx) 
    126154            return cnx 
    127155        # Third best option: Create a new connection 
    128156        elif len(self._active) + len(self._pool) < self._maxsize: 
    129             create = True 
     157            return ('create', None) 
    130158        # Forth best option: Replace a pooled connection with a new one 
    131159        elif len(self._active) < self._maxsize: 
    132160            # Remove the LRU connection in the pool 
    133             self._pool.pop(0).close() 
     161            self._pool.pop(0) 
    134162            self._pool_key.pop(0) 
    135163            self._pool_time.pop(0) 
    136             create = True 
    137         if create: 
    138             return connector.get_connection(**kwargs) 
     164            return ('close', None) 
    139165 
    140166    def _return_cnx(self, cnx, key, tid): 
     167        # Decrement active refcount, clear slot if 1 
    141168        self._available.acquire() 
    142169        try: 
    143170            assert (tid, key) in self._active 
    144171            cnx, num = self._active[(tid, key)] 
    145172            if num == 1: 
    146173                del self._active[(tid, key)] 
    147                 self._available.notify()  
    148                 if cnx.poolable and try_rollback(cnx): 
    149                     self._pool.append(cnx) 
    150                     self._pool_key.append(key) 
    151                     self._pool_time.append(time.time()) 
    152174            else: 
    153175                self._active[(tid, key)] = (cnx, num - 1) 
    154176        finally: 
    155177            self._available.release() 
     178        if num == 1: 
     179            # Reset connection outside of critical section 
     180            if not try_rollback(cnx): # TODO inline this in 0.13 
     181                cnx = None 
     182            # Connection available, from reuse or from creation of a new one 
     183            self._available.acquire() 
     184            try: 
     185                if cnx and cnx.poolable: 
     186                    self._pool.append(cnx) 
     187                    self._pool_key.append(key) 
     188                    self._pool_time.append(time.time()) 
     189                self._available.notify()  
     190            finally: 
     191                self._available.release() 
    156192 
    157193    def shutdown(self, tid=None): 
    158194        """Close pooled connections not used in a while"""