Skip to content

Conversation

mxschmitt
Copy link
Member

Fix memory leak where ProtocolCallback objects accumulated when tracing was enabled. The issue affected both sync and async Playwright, but was more severe in async usage where callbacks persisted permanently.

Root Cause

  • When tracing is enabled, send_no_reply() creates ProtocolCallback objects
  • These callbacks have circular references via task cancellation handlers
  • For no_reply messages, futures are never resolved, preventing GC cleanup
  • Each page operation during tracing leaked ~10 callbacks permanently

Solution

  • Add no_reply parameter to ProtocolCallback constructor
  • For no_reply=True: skip cancellation setup and resolve future immediately
  • For no_reply=False: use full constructor with cancellation handling
  • Update dispatch() to handle missing callbacks gracefully

Testing

  • Added regression test in test_reference_count_async.py
  • Test fails before fix (30+ leaked callbacks) and passes after (≤5)
  • All existing tracing and asyncio tests continue to pass

Fixes #2977

@mxschmitt mxschmitt force-pushed the fix/protocol-callback-memory-leak branch 2 times, most recently from 1e5fcaf to 714207c Compare September 29, 2025 08:40
Copy link
Contributor

@dgozman dgozman left a comment

Choose a reason for hiding this comment

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

Looks good, but bots are not happy.

Fix memory leak where ProtocolCallback objects accumulated when tracing
was enabled. The issue affected both sync and async Playwright, but was
more severe in async usage where callbacks persisted permanently.

- When tracing is enabled, send_no_reply() creates ProtocolCallback objects
- These callbacks have circular references via task cancellation handlers
- For no_reply messages, futures are never resolved, preventing GC cleanup
- Each page operation during tracing leaked ~10 callbacks permanently

- Add no_reply parameter to ProtocolCallback constructor
- For no_reply=True: skip cancellation setup and resolve future immediately
- For no_reply=False: use full constructor with cancellation handling
- Update dispatch() to handle missing callbacks gracefully

- Added regression test in test_reference_count_async.py
- Test fails before fix (30+ leaked callbacks) and passes after (≤5)
- All existing tracing and asyncio tests continue to pass

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
@mxschmitt mxschmitt force-pushed the fix/protocol-callback-memory-leak branch from 714207c to 90f062e Compare October 6, 2025 20:57
@mxschmitt mxschmitt merged commit 019df4b into main Oct 7, 2025
34 of 38 checks passed
@mxschmitt mxschmitt deleted the fix/protocol-callback-memory-leak branch October 7, 2025 13:36
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.

[Bug]: Object leak in Tracing
2 participants