Opened 10 years ago

Last modified 9 years ago

#4284 defect reopened

t.e.adbapi leaks threads if you don't explicitly call .close()

Reported by: Dustin J. Mitchell Owned by:
Priority: normal Milestone:
Component: core Keywords: doc enterprise adbapi
Cc: zooko@… Branch:


My project is using sqlite3 via adbapi, so the unit tests create a lot of "test" databases that are immediately destroyed.

These tests started failing with resource-exhaustion errors -- MemoryError or "can't start new thread" -- so I went hunting for a thread leak. I narrowed the search down to adbapi, and then to the threadpool. This is in Twisted-9.0.0. The failing test is

import threading
from twisted.python import threadpool
from twisted.internet import threads
from twisted.trial import unittest
class t(unittest.TestCase):
    def setUp(self):
        self.pool = threadpool.ThreadPool(3, 5)
    def tearDown(self):
        print list(threading.enumerate())
    def testit(self):
        from twisted.internet import reactor
        return threads.deferToThreadPool(reactor, self.pool, lambda : True)
    def testit2(self):
        return self.testit()
    def testit3(self):
        return self.testit()
    def testit4(self):
        return self.testit()
    def testit5(self):
        return self.testit()
    def testit6(self):
        return self.testit()
    def testit7(self):
        return self.testit()
    def testit8(self):
        return self.testit()

This shows that the number of extant threads is increasing by 3 for each test.

I've seen this under Python 2.4-2.6. I've only tried Twisted-9.0.0, so I don't know if this is a regression or not.

Change History (4)

comment:1 Changed 10 years ago by Dustin J. Mitchell

Resolution: invalid
Status: newclosed
Summary: t.p.threadpool leaks threadst.e.adbapi leaks threads

Ah, I'm such a fool - I need to be calling pool.stop! With twisted.enterprise.adbapi.ConnectionPool, I need to be calling pool.close, even though I do *not* need to call pool.start.

comment:2 Changed 9 years ago by zooko

Cc: zooko@… added
Keywords: doc enterprise adbapi added
Resolution: invalid
Status: closedreopened
Summary: t.e.adbapi leaks threadst.e.adbapi leaks threads if you don't explicitly call .close()

We just took down for a few minutes by constructing a new ConnectionPool object for each request. Okay, so that wasn't the right thing to do and we've now fixed it by holding a single ConnectionPool object for multiple requests.

But, why didn't the deletion of the old ConnectionPool object trigger it to close its connections?

I'm not in favor of finalizers in general, so if the answer is "Because finalizers aren't really guaranteed to run when you need them to run and in fact are a useless idea that should be omitted from future programming languages.", then perhaps a small patch to the documentation is in order to remind the reader to call .close() when they are done.

comment:3 Changed 9 years ago by zooko

USSJoin suggested the tutorial: which he read as a good place to mention "... and call .close() when you are done". I also thought maybe the docstring of ConnectionPool, ConnectionPool.start(), or ConnectionPool.__init__() would be appropriate.

comment:4 Changed 9 years ago by <automation>

Owner: Glyph deleted
Note: See TracTickets for help on using tickets.