-
Notifications
You must be signed in to change notification settings - Fork 187
Open
Labels
bugSomething isn't workingSomething isn't workingenhancementNew feature or requestNew feature or request
Description
Problem Description
The MplexStream implementation has deadline functionality methods (set_deadline, set_read_deadline, set_write_deadline) but the deadlines are never actually enforced in read/write operations. This means operations can hang indefinitely, leading to:
- Resource leaks: Connections that never timeout
- Unresponsive applications: Apps that freeze waiting for network data
- Poor user experience: No feedback when network operations fail
- API inconsistency: Interface promises functionality that doesn't work
Current State
Location: libp2p/stream_muxer/mplex/mplex_stream.py:244-271
The implementation has:
- β Deadline storage: self.read_deadline and self.write_deadline attributes
- β Deadline setting: Methods to set deadlines
- β Deadline enforcement: No timeout logic in read() and write() methods
- β Timeout checking: Operations can hang forever
Code Evidence
The TODO comment clearly indicates the issue:
# TODO deadline not in use
def set_deadline(self, ttl: int) -> bool:
self.read_deadline = ttl # Sets the deadline
self.write_deadline = ttl # Sets the deadline
return True
But the read() method has no deadline checking, so operations can hang forever.
Expected Behavior
Users should be able to set timeouts and get predictable behavior:
stream = mplex_stream
stream.set_deadline(30) # 30 second timeout
try:
data = await stream.read(1000) # Should timeout after 30s
except TimeoutError:
print("Read operation timed out")
Impact
- High: API promises functionality that doesn't work
- Medium: Users may expect timeouts to work and be confused when they don't
- Low: Implementation effort - mostly adding deadline checks to existing methods
Proposed Solution
Implement deadline checking in read/write operations:
- Add deadline validation before operations
- Use trio.fail_after() for timeout enforcement
- Add appropriate exceptions for timeout scenarios
- Update tests to cover deadline functionality
Related Issues
- This is Issue added linter and style fixesΒ #4 from the analysis in discussion_723.md
- Related to stream state management in PR feat: Adding stream state to the Network StreamΒ #637 (though that doesn't address this specific issue)
Priority
High Priority: This is a straightforward enhancement that would complete the API's promised functionality and improve user experience for production applications.
Metadata
Metadata
Assignees
Labels
bugSomething isn't workingSomething isn't workingenhancementNew feature or requestNew feature or request