Skip to content

Revert broken async sleep helpers in execution_mixin.py #1279

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.

Sign up for GitHub

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

Merged
MervinPraison merged 3 commits into from
Apr 7, 2026
Merged

Revert broken async sleep helpers in execution_mixin.py #1279

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
efb92fd
Fix blocking time.sleep() calls in async code paths
MervinPraison Apr 5, 2026
ecfd858
Remove broken _smart_sleep/_async_safe_sleep helpers; restore time.sl…
Copilot Apr 5, 2026
7cc9718
Fix missing Mount import in MCP server launch
MervinPraison Apr 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 45 additions & 6 deletions src/praisonai-agents/praisonaiagents/agent/execution_mixin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,45 @@
import asyncio
import threading


def _async_safe_sleep(seconds: float):
"""Async-safe sleep that uses asyncio.sleep when in an event loop."""
try:
# Check if there's a running event loop
loop = asyncio.get_running_loop()
if loop.is_running():
# We're in an async context, use asyncio.sleep
return asyncio.create_task(asyncio.sleep(seconds))
except RuntimeError:
# No running event loop, we're in sync context
pass

# Fallback to regular time.sleep for sync contexts
time.sleep(seconds)
return None
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Apr 5, 2026

Choose a reason for hiding this comment

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

critical

The implementation of _async_safe_sleep is problematic for synchronous callers. When an event loop is running, it returns an asyncio.Task and continues execution immediately. Since the calling code in this file is synchronous and does not (and cannot) await this task, the execution flow is not actually paused. This is not a valid replacement for time.sleep() if the intention is to wait before proceeding to the next line of code.

Comment thread
qodo-code-review[bot] marked this conversation as resolved.
Outdated


def _smart_sleep(seconds: float):
"""Smart sleep that detects async context and uses appropriate sleep method.

Returns a coroutine if in async context, otherwise sleeps synchronously.
"""
try:
# Check if there's a running event loop
loop = asyncio.get_running_loop()
if loop.is_running():
# We're in an async context, return a coroutine
async def async_sleep():
await asyncio.sleep(seconds)
return async_sleep()
except RuntimeError:
# No running event loop, we're in sync context
pass

# Sync context - use regular time.sleep
time.sleep(seconds)
return None
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Apr 5, 2026

Choose a reason for hiding this comment

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

critical

The _smart_sleep function returns a coroutine object when an event loop is detected. However, all the call sites in this PR are synchronous and do not await the result. Consequently, in any environment where an event loop is running (e.g., an async application using this library), the sleep will be skipped entirely. This breaks the logic of waiting for server registration and, most critically, leads to high-CPU busy loops in the keep-alive sections of the code.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 5, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Returned coroutine is never awaited, sleep won't occur in async contexts.

When a running event loop is detected, this function returns a coroutine object but the caller never awaits it. Since all call sites (lines 995, 998, 1027, 1036, 1095, 1100) are in sync methods that simply call _smart_sleep(...) without await, the sleep is silently skipped and Python emits a RuntimeWarning: coroutine 'async_sleep' was never awaited.

To properly block synchronously while not blocking the event loop, use run_coroutine_threadsafe:

🐛 Proposed fix using run_coroutine_threadsafe 
 def _smart_sleep(seconds: float):
     """Smart sleep that detects async context and uses appropriate sleep method.
     
-    Returns a coroutine if in async context, otherwise sleeps synchronously.
+    Sleeps without blocking the event loop when called from a sync context
+    while an async loop is running in another thread.
     """
     try:
         # Check if there's a running event loop
         loop = asyncio.get_running_loop()
         if loop.is_running():
-            # We're in an async context, return a coroutine
-            async def async_sleep():
-                await asyncio.sleep(seconds)
-            return async_sleep()
+            # Schedule async sleep on the running loop and wait for completion
+            future = asyncio.run_coroutine_threadsafe(asyncio.sleep(seconds), loop)
+            future.result()  # Block until sleep completes
+            return
     except RuntimeError:
         # No running event loop, we're in sync context
         pass
     
     # Sync context - use regular time.sleep
     time.sleep(seconds)
-    return None

Based on learnings: "All I/O operations must have both sync and async variants; never block the event loop with sync I/O in async context."

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/praisonai-agents/praisonaiagents/agent/execution_mixin.py` around lines
35 - 54, _smart_sleep currently returns an un-awaited coroutine when an event
loop is running, causing the sleep to be skipped; change its async-branch to
schedule the sleep on the running loop using asyncio.run_coroutine_threadsafe
and block until completion (e.g., call
asyncio.run_coroutine_threadsafe(asyncio.sleep(seconds), loop).result()),
catching and falling back to time.sleep on errors; reference the _smart_sleep
function, asyncio.get_running_loop(), asyncio.sleep and
asyncio.run_coroutine_threadsafe so you can locate the change, and ensure
exceptions from .result() are handled (and a synchronous time.sleep used as
fallback).


# Fallback helpers to avoid circular imports
def _get_console():
from rich.console import Console
Expand Down Expand Up @@ -953,10 +992,10 @@ def run_server():
server_thread.start()

# Wait for a moment to allow the server to start and register endpoints
time.sleep(0.5)
_smart_sleep(0.5)
else:
# If server is already running, wait a moment to make sure the endpoint is registered
time.sleep(0.1)
_smart_sleep(0.1)
print(f"🔌 Available endpoints on port {port}: {', '.join(list(_registered_agents[port].keys()))}")

# Get the stack frame to check if this is the last launch() call in the script
Expand Down Expand Up @@ -985,7 +1024,7 @@ def run_server():
try:
print("\nAll agents registered for HTTP mode. Press Ctrl+C to stop the servers.")
while True:
time.sleep(1)
_smart_sleep(1)
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Apr 5, 2026

Choose a reason for hiding this comment

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

critical

This while True loop will become a high-CPU busy loop in an asynchronous environment. Because _smart_sleep(1) returns a coroutine object immediately without awaiting it, the loop will iterate as fast as possible without any delay. Synchronous code that needs to pause execution must use a blocking sleep like time.sleep(). If the goal is to avoid blocking the event loop, the launch methods should be refactored to be async so they can properly await asyncio.sleep().

Suggested change
while True:
time.sleep(1)
_smart_sleep(1)
while True:
time.sleep(1)

Comment thread
qodo-code-review[bot] marked this conversation as resolved.
Outdated
except KeyboardInterrupt:
print("\nServers stopped")
except Exception as e:
Expand All @@ -994,7 +1033,7 @@ def run_server():
try:
print("\nKeeping HTTP servers alive. Press Ctrl+C to stop.")
while True:
time.sleep(1)
_smart_sleep(1)
except KeyboardInterrupt:
print("\nServers stopped")
return None
Expand Down Expand Up @@ -1053,12 +1092,12 @@ def run_mcp_server():

server_thread = threading.Thread(target=run_mcp_server, daemon=True)
server_thread.start()
time.sleep(0.5)
_smart_sleep(0.5)

try:
print("\nKeeping MCP server alive. Press Ctrl+C to stop.")
while True:
time.sleep(1)
_smart_sleep(1)
except KeyboardInterrupt:
print("\nMCP Server stopped")
return None
Expand Down
Loading