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

Only single instance of Tnz feasible #71

Closed
DNikolaevAtRocket opened this issue Jun 15, 2023 · 4 comments · Fixed by #73
Closed

Only single instance of Tnz feasible #71

DNikolaevAtRocket opened this issue Jun 15, 2023 · 4 comments · Fixed by #73
Assignees

Comments

@DNikolaevAtRocket
Copy link

DNikolaevAtRocket commented Jun 15, 2023

Description

We keep exploring the lib which really look promising. Thanks for doing it.

But the critical limitation we ran into is that it's not possible to get multiple fully functional instances of the Tnz class.
As far as I can see, the code uses global objects like _wait_event which breaks any possibility of that.

Generally, it'd be very nice forTnz to be fully thread-safe.

Simple demo of the issue

The code below causes a deadlock:

from tnz.tnz import Tnz


session_0 = Tnz()
session_1 = Tnz()

session_0.connect("<HOST_0>")
session_1.connect("<HOST_1>")

session_0.wait()
session_1.wait()  # This never returns
@najohnsn najohnsn self-assigned this Jun 26, 2023
@najohnsn
Copy link
Member

najohnsn commented Jun 30, 2023

You're expectations of wait() only waiting for a change for that Tnz instance is very reasonable. I will make that change. In order to make it, an interface needs to be provided so that a group of Tnz instances can share a single event used for wait() - ati will exploit this.

Threading was mentioned. Tnz uses the asyncio event loop to manage I/O. To use different Tnz instances in different threads means you would need a different asyncio event loop for each. I do not plan on forcing Tnz instances to have different asyncio event loops. That could change another potential expectation from the scenario in the description - that while waiting for one session, I/O is proceeding for other sessions. However, you can control the loop because connect() will call asyncio.get_event_loop(). That means that you can control the event loop used in each thread by setting it as the current ahead of time:

asyncio.set_event_loop(asyncio.new_event_loop())

@v1gnesh
Copy link

v1gnesh commented Jul 1, 2023

Hey, speaking of event loop... would you consider adding support for uvloop?

@najohnsn
Copy link
Member

najohnsn commented Jul 1, 2023

I wouldn't except there would be any advantage to using uvloop when running zti.

I've never used uvloop personally. But it is described as being a drop-in replacement for asyncio and it has instructions on how to enable it by setting it as the default event loop policy. So, your program should be able to do that and then tnz should use it.

If there is some issue with using uvloop and tnz together, I would welcome an Issue for it. But I would not expect tnz to depend on uvloop.

@najohnsn
Copy link
Member

najohnsn commented Jul 3, 2023

I decided that requiring a tnz.new_asyncio_event_loop() would be too complicated. So, I decided to add a commit to the PR that will allow tnz to work - even for the default Windows ProactorEventLoop (which doesn't support add_reader()). I'm changing a previous comment to remove references to that function.

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 a pull request may close this issue.

3 participants