Skip to content

Conversation

@jaiganeshs21
Copy link
Contributor

@jaiganeshs21 jaiganeshs21 commented Jul 5, 2025

Summary

This PR fixes a critical memory leak in Celery's exception handling that was causing significant memory growth when tasks raise unhandled exceptions.

Problem

Issue #8882 reported that Celery tasks raising unhandled exceptions were causing memory leaks:

  • Memory not being garbage collected due to reference cycles
  • Worse on Python 3.11+ due to enhanced traceback data

Closes #8882

This PR addresses a critical memory leak in Celery's exception handling by breaking reference cycles in tracebacks, ensuring frame locals are cleared, and adding comprehensive tests to verify the fix.

  • Add traceback_clear calls and explicit deletion of traceback references in retry, failure, and signal-handling paths
    
  • Enhance traceback_clear to efficiently clear frame data and update its docstring
    
  • Introduce unit and integration tests to validate that tracebacks no longer retain memory across various exception scenarios
    

@jaiganeshs21 jaiganeshs21 force-pushed the fix-memory-leak-issue-8882 branch from dd72947 to 33c7a22 Compare July 5, 2025 08:56
@codecov
Copy link

codecov bot commented Jul 5, 2025

Codecov Report

❌ Patch coverage is 88.23529% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.62%. Comparing base (a83070e) to head (ca2d222).
⚠️ Report is 44 commits behind head on main.

Files with missing lines Patch % Lines
celery/app/base.py 60.00% 3 Missing and 1 partial ⚠️
celery/app/trace.py 92.85% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9799      +/-   ##
==========================================
- Coverage   78.62%   78.62%   -0.01%     
==========================================
  Files         153      153              
  Lines       19178    19201      +23     
  Branches     2540     2547       +7     
==========================================
+ Hits        15078    15096      +18     
- Misses       3809     3812       +3     
- Partials      291      293       +2     
Flag Coverage Δ
unittests 78.60% <88.23%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

- Enhanced traceback cleanup in celery/app/trace.py to prevent memory leaks
- Added proper cleanup of ExceptionInfo objects and traceback references
- Optimized traceback_clear() function by removing redundant f_locals access
- Added comprehensive memory leak test suite in t/integration/test_memory_leak_8882.py
- Fixed code quality issues: removed unused imports, cleaned whitespace, added noqa comments

Memory usage improvement: 92% reduction (from ~70MB to ~0.6MB for 500 failing tasks)
Addresses reference cycles that prevent garbage collection of traceback frames.
All pre-commit hooks passing.
@jaiganeshs21 jaiganeshs21 force-pushed the fix-memory-leak-issue-8882 branch from a749037 to 1ca9ff4 Compare July 5, 2025 09:04
@auvipy auvipy requested review from auvipy and Copilot July 5, 2025 09:41
@auvipy auvipy added this to the 5.6.0 milestone Jul 5, 2025

This comment was marked as outdated.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@auvipy auvipy mentioned this pull request Jul 5, 2025
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@jaiganeshs21
Copy link
Contributor Author

Thanks @auvipy for adding the Copilot review and providing feedback. I've addressed the comments. Let me know if you have any other suggestions. Thanks!

@auvipy auvipy requested a review from Copilot July 5, 2025 11:07

This comment was marked as outdated.

@auvipy
Copy link
Member

auvipy commented Jul 5, 2025

btw unit tests are separated from integration tests

@jaiganeshs21
Copy link
Contributor Author

btw unit tests are separated from integration tests

Got it, Separated the unit and integration tests and moved them to their respective locations.
Commit - f2e24b3

@auvipy auvipy requested a review from Copilot July 5, 2025 14:40

This comment was marked as outdated.

@jaiganeshs21 jaiganeshs21 requested a review from Copilot July 5, 2025 15:25

This comment was marked as outdated.

@jaiganeshs21 jaiganeshs21 requested a review from Copilot July 5, 2025 15:27

This comment was marked as outdated.

@jaiganeshs21 jaiganeshs21 requested a review from Copilot July 5, 2025 15:29

This comment was marked as outdated.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@jaiganeshs21 jaiganeshs21 requested a review from Copilot July 5, 2025 15:32

This comment was marked as outdated.

@jaiganeshs21 jaiganeshs21 requested a review from Copilot July 5, 2025 15:33

This comment was marked as outdated.

@jaiganeshs21 jaiganeshs21 requested a review from Copilot July 5, 2025 15:38

This comment was marked as outdated.

@jaiganeshs21 jaiganeshs21 requested a review from Copilot July 5, 2025 15:40

This comment was marked as resolved.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Member

@auvipy auvipy left a comment

Choose a reason for hiding this comment

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

overall looks good

This comment was marked as resolved.

@auvipy auvipy merged commit 4c7443d into celery:main Jul 7, 2025
124 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Memory Leak on Unhandled Exceptions

2 participants