Skip to content

Commit

Permalink
Handle __del__ of connection object more gracefully #390
Browse files Browse the repository at this point in the history
--------
Co-authored-by: Christoph Pirkl <christoph.pirkl@exasol.com>
  • Loading branch information
Nicoretti authored Nov 3, 2023
1 parent f7be0ea commit 39d7f13
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 3 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@
Unreleased
==========

🐞 Fixed
---------

- Fixed `WebSocket connection isn't properly closed in case of process termination <https://github.com/exasol/pyexasol/issues/108>`_

.. _changelog-4.6.0:

4.6.0 — 2023-07-18
Expand Down
20 changes: 17 additions & 3 deletions exasol/driver/websocket/_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,12 @@ def connection(self):

def close(self):
"""See also :py:meth: `Connection.close`"""
if not self._connection:
connection_to_close = self._connection
self._connection = None
if connection_to_close is None or connection_to_close.is_closed:
return
try:
self._connection.close()
connection_to_close.close()
except Exception as ex:
raise Error() from ex

Expand All @@ -148,4 +150,16 @@ def cursor(self):
return DefaultCursor(self)

def __del__(self):
self.close()
if self._connection is None:
return

# Currently, the only way to handle this gracefully is to invoke the`__del__`
# method of the underlying connection rather than calling an explicit `close`.
#
# For more details, see also:
# * https://github.com/exasol/sqlalchemy-exasol/issues/390
# * https://github.com/exasol/pyexasol/issues/108
#
# If the above tickets are resolved, it should be safe to switch back to using
# `close` instead of `__del__`.
self._connection.__del__()
37 changes: 37 additions & 0 deletions test/integration/regression/test_regression_bug390.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
from inspect import cleandoc

pytest_plugins = "pytester"


def test_connection_with_block_cleans_up_properly(pytester, exasol_config):
config = exasol_config
# Because the error only occurs on process shutdown we need to run a test within a test
# (We require the result (stderr) of a terminated process triggering the failure.
pytester.makepyfile(
# fmt: off
cleandoc(
f"""
from sqlalchemy import create_engine
def test():
url = "exa+websocket://{{user}}:{{pw}}@{{host}}:{{port}}/{{schema}}?SSLCertificate=SSL_VERIFY_NONE"
url = url.format(
user="{config.username}",
pw="{config.password}",
host="{config.host}",
port={config.port},
schema="TEST",
)
engine = create_engine(url)
query = "SELECT 42;"
with engine.connect() as con:
result = con.execute(query).fetchall()
"""
),
# fmt: on
)
r = pytester.runpytest_subprocess()
expected = ""
actual = str(r.stderr)

assert actual == expected

0 comments on commit 39d7f13

Please sign in to comment.