Skip to content

Commit cb52165

Browse files
Close leaked sqlite3 connections (ResourceWarning on Python 3.13+) (#6181)
Python 3.13+ reports sqlite3 connections that are garbage collected without being closed. The unit tests print a run of "ResourceWarning: unclosed database" lines on Linux/CPython 3.14: - SQLAlchemy.close() only returned its connection to the engine's pool, so the underlying DBAPI connection (e.g. sqlite3 for '-d sqlite://...') stayed open. Dispose the engine on close. - vulnTest() used "with sqlite3.connect(...)", which commits but does not close the connection. Use contextlib.closing(). - test_multibit and test_sqlalchemy never closed what they opened. Co-authored-by: PiyushSatyarthi <244159432+PiyushSatyarthi@users.noreply.github.com>
1 parent eca9ff2 commit cb52165

4 files changed

Lines changed: 31 additions & 6 deletions

File tree

‎lib/core/testing.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
See the file 'LICENSE' for copying permission
66
"""
77

8+
import contextlib
89
import doctest
910
import json
1011
import logging
@@ -206,9 +207,11 @@ def _thread():
206207
os.close(handle)
207208
cleanups.append(database)
208209

209-
with sqlite3.connect(database) as conn:
210+
# Note: sqlite3's own context manager only commits (it does not close the connection)
211+
with contextlib.closing(sqlite3.connect(database)) as conn:
210212
c = conn.cursor()
211213
c.executescript(vulnserver.SCHEMA)
214+
conn.commit()
212215

213216
handle, request = tempfile.mkstemp(suffix=".req")
214217
os.close(handle)

‎lib/utils/sqlalchemy.py‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ def __init__(self, dialect=None):
5050

5151
self.dialect = dialect
5252
self.address = conf.direct
53+
self.engine = None
5354

5455
if conf.dbmsUser:
5556
self.address = self.address.replace("'%s':" % conf.dbmsUser, "%s:" % _urllib.parse.quote(conf.dbmsUser))
@@ -84,6 +85,7 @@ def connect(self):
8485
else:
8586
engine = _sqlalchemy.create_engine(self.address, connect_args={})
8687

88+
self.engine = engine
8789
self.connector = engine.connect()
8890
except (TypeError, ValueError) as ex:
8991
if "_get_server_version_info" in traceback.format_exc():
@@ -110,6 +112,19 @@ def connect(self):
110112
else:
111113
raise SqlmapMissingDependence("SQLAlchemy not available (e.g. 'pip%s install SQLAlchemy')" % ('3' if six.PY3 else ""))
112114

115+
def close(self):
116+
try:
117+
GenericConnector.close(self)
118+
finally:
119+
# Note: closing the connection only returns it to the engine's pool (the underlying DBAPI
120+
# connection, e.g. sqlite3, stays open) - disposing the engine is what actually releases it
121+
if self.engine is not None:
122+
try:
123+
self.engine.dispose()
124+
except Exception as ex:
125+
logger.debug(getSafeExString(ex))
126+
self.engine = None
127+
113128
def fetchall(self):
114129
try:
115130
retVal = []

‎tests/test_multibit.py‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -140,8 +140,14 @@ def tearDown(self):
140140
multibit._page = self._savedPage
141141
multibit.singleTimeWarnMessage, multibit.singleTimeLogMessage = _SAID
142142

143-
def _attempt(self, secret, length=None, **kwargs):
143+
def _target(self, secret, **kwargs):
144144
target = _Target(secret, **kwargs)
145+
self.addCleanup(target.db.close)
146+
147+
return target
148+
149+
def _attempt(self, secret, length=None, **kwargs):
150+
target = self._target(secret, **kwargs)
145151
multibit._page = target.page
146152

147153
return self._read(target, secret, length), target
@@ -231,7 +237,7 @@ def test_first_value_that_cannot_settle_the_cross_check_keeps_the_channel(self):
231237
# non-ASCII one, and one whose codepoint has an ASCII low byte (U+0141 -> 0x41 'A') all settle
232238
# nothing, and none of them may condemn a channel that calibrated cleanly
233239
for first in ("", u"\u017eaba pod vodom", u"\u0141ukasz Nowak here"):
234-
target = _Target(first)
240+
target = self._target(first)
235241
self.assertIsNone(self._read(target, first, len(first) or None))
236242

237243
good = target.secret("plain ascii value that must still come back")
@@ -242,7 +248,7 @@ def test_null_values_do_not_condemn_the_channel(self):
242248
# what the channel did - i.e. exactly the way a channel dropping rows fails it. That is the
243249
# data's doing, and a handful of NULLs in one dump must not retire a working channel
244250
first = "first value is perfectly readable ascii"
245-
target = _Target(first)
251+
target = self._target(first)
246252
self.assertEqual(self._read(target, first, len(first)), first)
247253

248254
for _ in range(MULTIBIT_MAX_FAILURES + 1):
@@ -253,7 +259,7 @@ def test_null_values_do_not_condemn_the_channel(self):
253259
self.assertEqual(self._read(target, good, len(good)), good)
254260

255261
def test_unreadable_value_mid_run_keeps_the_channel(self):
256-
target = _Target("first value is perfectly readable ascii")
262+
target = self._target("first value is perfectly readable ascii")
257263
self.assertEqual(self._read(target, "first value is perfectly readable ascii", 39), "first value is perfectly readable ascii")
258264

259265
bad = target.secret(u"\u0141ukasz") # reads back as "Aukasz" - right length, wrong char
@@ -295,7 +301,7 @@ def test_nothing_runs_without_the_switch(self):
295301
def test_resumed_partial_value_is_left_to_bisection(self):
296302
# a resumed value starts mid-string, and multi-bit reads from the first character: it would
297303
# re-read what is already cached and then fail its own length read-back
298-
target = _Target("resuming this one is bisection's job")
304+
target = self._target("resuming this one is bisection's job")
299305
multibit._page = target.page
300306

301307
self.assertIsNone(multibit.attempt("SELECT s FROM v", lambda idx: 'x', 35, None, True))

‎tests/test_sqlalchemy.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ def test_absolute_sqlite_path_opens_correct_file(self):
5858

5959
sa = SQLAlchemy(dialect="sqlite")
6060
sa.connect()
61+
self.addCleanup(sa.close)
6162

6263
# the reformatted URL must resolve to the exact absolute file (not '//...path')
6364
self.assertEqual(_sa.engine.url.make_url(sa.address).database, os.path.abspath(dbfile))

0 commit comments

Comments
 (0)