Skip to content

Commit

Permalink
[Safe Client disconnect]
Browse files Browse the repository at this point in the history
Follow-up of #711
Make sure calling disconnect is safe when connection isn't established
or half-established (i.e: protocol/transport exist but session is not
ready).

Test1
Call to disconnect is safe when there's no existing connection
```
> python -m IPython
Python 3.10.0 (default, Oct 13 2021, 06:45:00) [Clang 13.0.0 (clang-1300.0.29.3)]

In [1]: import sys; sys.path.insert(0, "~/Documents/github/opcua-asyncio"); import asyncua; c = asyncua.Client(url="opc.tcp://localhost:4840",timeout=10
   ...: );

In [2]: await c.disconnect()
close_session but connection wasn't established
close_secure_channel was called but connection is closed

In [3]:
```
Test2

Calling to disconnect is safe when there's no session established.
To simulate this, I add delay to the server internal_session on session
creation. I check that the client connects and disconnect from the
server logs. Finally, the client doesn't raise any error.
Ad delay to the internal server
```
> python -m IPython
Python 3.10.0 (default, Oct 13 2021, 06:45:00) [Clang 13.0.0 (clang-1300.0.29.3)]
In [1]: import sys; sys.path.insert(0, "~/Documents/github/opcua-asyncio"); import asyncua; c = asyncua.Client(url="opc.tcp://localhost:4840",timeout=10
   ...: ); import threading; import asyncio;

In [2]:

In [2]: async def connect(c):
   ...:     await c.connect()
   ...:

In [3]: async def disco(c):
   ...:     await c.disconnect()
   ...:

In [4]: L = await asyncio.gather(connect(c), disco(c))
close_session but connection wasn't established
close_secure_channel was called but connection is closed

In [5]: quit;
```
  • Loading branch information
Julien Prigent authored and oroulet committed Nov 12, 2021
1 parent b5edbde commit 560eb91
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 6 deletions.
11 changes: 6 additions & 5 deletions asyncua/client/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -513,11 +513,12 @@ async def close_session(self) -> Coroutine:
"""
Close session
"""
self._renew_channel_task.cancel()
try:
await self._renew_channel_task
except Exception:
_logger.exception("Error while closing secure channel loop")
if self._renew_channel_task:
self._renew_channel_task.cancel()
try:
await self._renew_channel_task

This comment has been minimized.

Copy link
@HarrySky

HarrySky Nov 29, 2021

Contributor

This will not catch asyncio.CancelledError on Python 3.8+, since Python 3.8 asyncio.CancelledError is subclass of BaseException.

except Exception:
_logger.exception("Error while closing secure channel loop")
return await self.uaclient.close_session(True)

def get_root_node(self):
Expand Down
5 changes: 4 additions & 1 deletion asyncua/client/ua_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ async def close_secure_channel(self):
close secure channel. It seems to trigger a shutdown of socket
in most servers, so be prepare to reconnect
"""
if self.protocol and self.protocol.state == UASocketProtocol.CLOSED:
if not self.protocol or self.protocol.state == UASocketProtocol.CLOSED:
self.logger.warning("close_secure_channel was called but connection is closed")
return
return await self.protocol.close_secure_channel()
Expand Down Expand Up @@ -313,6 +313,9 @@ async def activate_session(self, parameters):

async def close_session(self, delete_subscriptions):
self.logger.info("close_session")
if not self.protocol:
self.logger.warning("close_session but connection wasn't established")
return
self.protocol.closed = True
if self._publish_task and not self._publish_task.done():
self._publish_task.cancel()
Expand Down
4 changes: 4 additions & 0 deletions tests/test_connections.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,7 @@ async def test_max_connections_1(opc):
async with Client(f'opc.tcp://127.0.0.1:{port}'):
pass
opc.server.iserver.isession.__class__.max_connections = 1000

async def safe_disconnect():
c = Client(url="opc.tcp://example:4840")
await c.disconnect()

0 comments on commit 560eb91

Please sign in to comment.