Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Sync Client and Server reusable #1503

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

miki5799
Copy link

As the issue #1364 suggests, OpcUa Client (and Server) are not reusable, since disconnecting the client or stopping the server, stops the Sync Wrapper's ThreadLoop respectively and therefore lead to ThreadLoopNotRunnning exceptions on any call that depends on the ThreadLoop.

As in the discussion of #1364 the Sync Wrapper should start the ThreadLoop on connection (or server.start) if ThreadLoop is not alive anymore.
This proposed behaviour has been implemented in this PR.

Reopen ThreadLoop if  stopped previously analogous to init
Fixes FreeOpcUa#1364
self.tloop = ThreadLoop()
self.tloop.start()
self.close_tloop = True
self.tloop.post(self.aio_obj.connect())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we cannot have the same code several places. At least put that code in a method.
also if you want to delay threadloop initializatin, you might need to do it at several places not only here... there is a connect_sessionless method for example and maybe others...

self.tloop = ThreadLoop()
self.tloop.start()
self.close_tloop = True
self.tloop.post(self.aio_obj.start())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

arrg same code again... Making that class re-usable might not be a good idea. there is probably a reason, threading.Thread is not reusable...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your time and effort, but could you clarify further, since I'm not quite sure in what way initializing a new Thread or ThreadLoop could pose a problem in this context.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was simply refering to the fact the threading.Thread in the python standard library does not allow re-use. We copied their API when creating that class. We can make that class reusable, but I see your code over is not really clean, so maybe it is hard to solve that issue in a safe way...

@miki5799 miki5799 marked this pull request as draft December 18, 2023 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants