2

I have a class which opens a SQLite database in it's __init__ method and i want to close it later by using the build-in del function on the created object.

My destructor looks like this:

def __del__(self):
    self.connector.close()

Is this a secure way to close the connection to the database?

jervis
  • 151
  • 1
  • 1
  • 9
  • 1
    I don't think security is the issue. I think the issue is that [it might never close the connection](http://stackoverflow.com/questions/6104535/i-dont-understand-this-python-del-behaviour#answer-6104568). – Alex W Apr 09 '14 at 18:22
  • 2
    A better way would be to use context managers. – roippi Apr 09 '14 at 18:25
  • @AlexW: Ok, i was more worried about the idea that it could close unexpected. But this point of view helps me as well. – jervis Apr 09 '14 at 18:26
  • @roippi: I think just adding a method (to the class) which closes the connection would also be ok? – jervis Apr 09 '14 at 18:29
  • @jervis I thought when you said 'secure' you meant that it will be close **for sure**. Anyway, I answered as it this was the case :) – logc Apr 09 '14 at 18:31
  • @logc: You are right, my definition of secure in this case is that it gets closed when i want to, and dont gets closed by accident. – jervis Apr 09 '14 at 18:33

2 Answers2

2

No, the destructor will not get called if there is an exception while this object is still alive. Check this other SO question.

As noted there, you should change your class into a context manager, so that it can be used in a with statement. This ensures that its __exit__ method will always get called.

You can also look into the contextlib module, which makes it easier to produce such context managers.

Community
  • 1
  • 1
logc
  • 3,813
  • 1
  • 18
  • 29
  • 1
    It's not so good a solution if you need to keep the connection (not transaction) alive for longer than a logical context block. Rebuilding database connections is relatively expensive, where as opening transactions is not. – aruisdante Apr 09 '14 at 18:33
  • @aruisdante: Why isn't it a good solution? In my case i want to keep a SQLite connection open for a long time. – jervis Apr 09 '14 at 18:38
  • 1
    Because the entire life cycle of that database object must be contained within the ``with x as y:`` block. This is fine if you're only going to connect to a single database at the very start of your program, but otherwise impractical. The alternative, rebuilding a database connection every time you want to access the database, is very expensive computationally. – aruisdante Apr 09 '14 at 18:43
  • 1
    Generally, you wrap the database connection in a life-cycle management object that provides a ``context manager`` for acquiring a *transaction* and committing/rolling back during its exit, and then provides a ``close`` method to close the actual connection when you want it to. In the event of an expected failure (I.E. uncaught exception), the underlying ``SQLite`` library is properly tearing down the connection, you don't need to worry about making sure it does so. – aruisdante Apr 09 '14 at 18:47
  • 1
    But making sure *transactions* are properly committed is very important because ``SQLite`` locks the database at the beginning of a transaction and until a ``commit`` or ``rollback`` is called. Hence why you want a context manager for them. – aruisdante Apr 09 '14 at 18:48
  • @aruisdante: Is the creation of a life-cycle management object possible with pythons standard library? – jervis Apr 09 '14 at 19:00
  • 1
    @jervis Not that I know of. You can however if you're really worried about it wrap the entire program in a ``try: finally:`` block that closes connections during the finally without trapping the exception (so that it's still raised). You can also use [atexit](https://docs.python.org/2/library/atexit.html) to ensure they're closed if the program is killed externally. But again, the ``SQLite`` library should do that for you on program termination. – aruisdante Apr 09 '14 at 19:03
  • 1
    @aruisdante - the best way to handle connections is to have a pool of reusable connections (e.g. using SQLAlchemy or another ORM). However, the OP wants to know the drawbacks of closing the connection on simple object destruction -- I think an ORM as a solution is overreaching. But moreover, wrapping everything in a `try: finally:` or using `atexit` is quite the same as using a `with` statement. So I don't know what exactly are you proposing. :) – logc Apr 09 '14 at 19:16
  • @logc the ``with`` would only let you have one connection to one database. If the OP is even bothering to want to have lifecycle management on the connection, I assume they want to have more than one. the ``try: finally`` would let you have a simple conneciton manager at that scope that the rest of the program could add connection objects to, and then at the ``finally`` close them. I agree though that using something like SQLAlchemy is really the better option long-term, but if the OP doesn't want/can't use one, they can at least duplicate some of the basic connection-management functionality. – aruisdante Apr 10 '14 at 00:18
  • @logc You could of course make a ``contextmanager`` out of the simple connection manager and use the ``with`` on that ;) – aruisdante Apr 10 '14 at 00:19
0

Yes, closing the connection in the __del__() method is very dubious. When and if that method is called is the decision of the garbage collector. It may never get called, or may not get called until well after you have established another connection to the database.

You must call the close() method yourself. I generally do this by adding a close() method to my connection-wrapping class and ensuring that it gets called using try / finally.

I have found it necessary in at least one application to make certain that I call close() in addition to commit() or rollback() when I'm done with a connection.

Larry Lustig
  • 49,320
  • 14
  • 110
  • 160