Work at SourceForge, help us to make it a better place! We have an immediate need for a Support Technician in our San Francisco or Denver office.

Close

#281 Error in DBAPI.close ()

closed-fixed
Oleg Broytman
None
5
2013-07-07
2013-06-05
Patrick Gendron
No

When closing the connection to a database, we often receive the following error:

UnboundLocalError: local variable 'conn' referenced before assignment

Looking at the code for the DBAPI class, we see the following error:

sqlobject/dbconnection.py:

def close(self):
    if not hasattr(self, '_pool'):
        # Probably there was an exception while creating this
        # instance, so it is incomplete.
        return
    if not self._pool:
        return
    self._poolLock.acquire()
    try:
        conns = self._pool[:]
        self._pool[:] = []
        for conn in conns:
            try:
                conn.close()
            except self.module.Error:
                pass
        del conn   <<<  MISSING INDENT
        del conns
    finally:
        self._poolLock.release()

Discussion

  • Oleg Broytman
    Oleg Broytman
    2013-06-05

    • assigned_to: Oleg Broytman
     
  • Oleg Broytman
    Oleg Broytman
    2013-06-05

    Instead of deleting and reintroduced 'conn' at every loop I'd rather test 'conns' for emptyness:

    def close(self):
        if not hasattr(self, '_pool'):
            # Probably there was an exception while creating this
            # instance, so it is incomplete.
            return
        if not self._pool:
            return
        self._poolLock.acquire()
        try:
            conns = self._pool[:]
            if conns:
                self._pool[:] = []
                for conn in conns:
                    try:
                        conn.close()
                    except self.module.Error:
                        pass
                del conn
            del conns
        finally:
            self._poolLock.release()
    

    What do you think? Can you test this?

     
    Last edit: Oleg Broytman 2013-06-05
  • You are still iterating over all conn from the conns list but only deleting the last one since del conn is outside the loop. I have been running with my modification (del conn in the for loop) for a few months without problem.

     
    • Oleg Broytman
      Oleg Broytman
      2013-06-05

      Connections are closed with conn.close(). del only removes the name (one name, 'conn') from Python namespace. What's the point?

       
      Last edit: Oleg Broytman 2013-06-05
  • I see, then your correction looks good.

     
  • Oleg Broytman
    Oleg Broytman
    2013-06-10

    Just found there is the test

    if not self._pool:
        return
    

    It seems to be impossible not to run the 'for' loop, so 'conn' should always be defined.

    Do you use SQLObject in a multithreaded program? The only way for the error is AFAIUI if the test is performed in one thread, then lock acquired in another, _pool is cleared, than the first thread acquired the lock and stumble upon the empty _pool. In this case I propose to perform the test after acquiring the lock:

    def close(self):
        if not hasattr(self, '_pool'):
            # Probably there was an exception while creating this
            # instance, so it is incomplete.
            return
        self._poolLock.acquire()
        try:
            if not self._pool:
                return
            conns = self._pool[:]
            self._pool[:] = []
            for conn in conns:
                try:
                    conn.close()
                except self.module.Error:
                    pass
            del conn
            del conns
        finally:
            self._poolLock.release()
    

    What do you think? Can you test it?

     
    Last edit: Oleg Broytman 2013-07-07
  • Oleg Broytman
    Oleg Broytman
    2013-07-07

    • status: open --> closed-fixed
     
  • Oleg Broytman
    Oleg Broytman
    2013-07-07

    I fixed it my way. Committed in the revision r4612 in the trunk. Will be in the next release (1.5.0).