Opened 4 years ago

Last modified 4 years ago

#4284 defect reopened

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

Reported by: dustin Owned by:
Priority: normal Milestone:
Component: core Keywords: doc enterprise adbapi
Cc: zooko@… Branch:
Author: Launchpad Bug:

Description

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

http://gist.github.com/310039

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)
        self.pool.start()
 
    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 4 years ago by dustin

  • Resolution set to invalid
  • Status changed from new to closed
  • Summary changed from t.p.threadpool leaks threads to t.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 4 years ago by zooko

  • Cc zooko@… added
  • Keywords doc enterprise adbapi added
  • Resolution invalid deleted
  • Status changed from closed to reopened
  • Summary changed from t.e.adbapi leaks threads to t.e.adbapi leaks threads if you don't explicitly call .close()

We just took down http://simplegeo.com 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 4 years ago by zooko

USSJoin suggested the tutorial: http://www.opendocs.net/python/twisted/howto/enterprise.html 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 3 years ago by <automation>

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