diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 75b4e867..cf3570e7 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -3,6 +3,11 @@ Unreleased ========== +🐞 Fixed +--------- + +- Fixed `WebSocket connection isn't properly closed in case of process termination `_ + .. _changelog-4.6.0: 4.6.0 — 2023-07-18 diff --git a/exasol/driver/websocket/_connection.py b/exasol/driver/websocket/_connection.py index 20c1944b..dd239a45 100644 --- a/exasol/driver/websocket/_connection.py +++ b/exasol/driver/websocket/_connection.py @@ -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 @@ -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__() diff --git a/test/integration/regression/test_regression_bug390.py b/test/integration/regression/test_regression_bug390.py new file mode 100644 index 00000000..ad0fe804 --- /dev/null +++ b/test/integration/regression/test_regression_bug390.py @@ -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