Skip to content

Commit

Permalink
Fix test hangs
Browse files Browse the repository at this point in the history
  • Loading branch information
afullerx committed Apr 16, 2024
1 parent 755187c commit c23643b
Showing 1 changed file with 5 additions and 4 deletions.
9 changes: 5 additions & 4 deletions nicegui/outbox.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,11 @@ async def loop(self) -> None:

while not self._should_stop:
try:
try:
await asyncio.wait_for(self._enqueue_event.wait(), timeout=1.0)
except (TimeoutError, asyncio.TimeoutError):
continue
if not self._enqueue_event.is_set():
try:
await asyncio.wait_for(self._enqueue_event.wait(), timeout=1.0)
except (TimeoutError, asyncio.TimeoutError):
continue

if not self.client.has_socket_connection:
await asyncio.sleep(0.1)
Expand Down

4 comments on commit c23643b

@me21
Copy link
Contributor

@me21 me21 commented on c23643b Apr 16, 2024

Choose a reason for hiding this comment

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

I wonder why does this make a difference... Shouldn't Event.wait() return immediately if it's set?

@afullerx
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's because the asyncio.wait_for() creates a separate task and wraps it around self._enqueue_event.wait(). This means it doesn't get a chance to return until it's already being run in another task, even if the event was set to begin with.

@me21
Copy link
Contributor

@me21 me21 commented on c23643b Apr 16, 2024

Choose a reason for hiding this comment

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

Do you mean if there's another CPU-bound task it can prevent .wait() from running? Like explained there?https://stackoverflow.com/questions/74132015/asyncio-wait-for-doesnt-time-out-as-expected

But shouldn't it just take longer than hang up?

@me21
Copy link
Contributor

@me21 me21 commented on c23643b Apr 16, 2024

Choose a reason for hiding this comment

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

Sorry, I read your comment to this. Great job that you found the fix for this nasty issue! 👍

Please sign in to comment.