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

Potencial Deadlock fix #25

Merged
merged 3 commits into from
Nov 14, 2024
Merged

Potencial Deadlock fix #25

merged 3 commits into from
Nov 14, 2024

Conversation

Netzvamp
Copy link
Contributor

@Netzvamp Netzvamp commented Nov 9, 2024

Hey,

i've got a deadlock when i've tried to use "send" inside a handler, for example:

from genesis import Session

class MyCall:
    def __init__(self, session: Session):
        self.session = session
        self.session.on('CHANNEL_BRIDGE', self.on_bridge)
        ...

    async def on_bridge(self, event):
        filter_output = await self.session.send(f"filter Unique-ID {event['Other-Leg-Unique-ID']}")
        print(filter_output)
        ...

The print(filter_output) will never run, cause the consumer is still stuck in the on_bridge handler and doesn't process the answer from the send command.

So i pushed the handlers in it's own tasks with:

if iscoroutinefunction(handler):
      create_task(handler(event))
  else:
      create_task(to_thread(handler, event))

That works for me, but i'm not sure that there aren't any side effects i don't know about. ⚠️ ⚠️ ⚠️

And i've fixed some typos and type hints.

I've read into the "-> Awaitable[Something]" return values and you can skip that and just write "-> Something", cause the async already implies the "Awaitable".

@Otoru
Copy link
Owner

Otoru commented Nov 14, 2024

Hi,

Thanks for another contribution to the project, I'm glad to see more people using it.

Everything looks good to me and no tests broke. I'll approve the merge request.

@Otoru Otoru self-assigned this Nov 14, 2024
@Otoru Otoru merged commit 7ac47db into Otoru:main Nov 14, 2024
3 checks passed
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.

2 participants