-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Disable Logging panel when DEBUG is set to False to prevent memory leaks #563
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.
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
Conversation
can be called explicitly to ensure logging is initiated.
I think we have to bite the bullet and write a |
523aa53
to
417e83c
Compare
@aaugustin can you please review the changes? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one small comment.
@@ -15,13 +16,33 @@ | |||
|
|||
class LogCollector(ThreadCollector): | |||
|
|||
def __init__(self): | |||
ThreadCollector.__init__(self) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a particular reason for using ThreadCollecter
instead of super()
? If so, I think it should be documented.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to resolve conflicts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this needs to fix the merge conflicts, or not sure if the fixes are already into main branch?
Closing because of inactivity. Please resubmit if you still want to work on this. |
I upgraded django-debug-toolbar from 0.9.4. to 1.0.1, and I noticed my application was quickly leaking memory even when settings.DEBUG was set to False. Once I removed debug_toolbar from the list of applications, the problem disappeared.
Upon further investigation, I found that LoggingPanel was quietly collecting all my log messages and never releasing them.
This problem only occurs if the Django application is fully initialized (e.g. using runserver), since this causes the LoggingPanel to register its handlers. Here is a simple application that illustrates the problem:
Run this application with the default debug_toolbar settings, and then use 'top' to watch the memory grow without bound. The problem appears that the LoggingPanel does not use the enable_instrumentation() function normally called by the middleware, which does the DEBUG check beforehand.
I think the simplest fix is to check if DEBUG is enabled before initializing the LogCollector. This pull request does just that, and it appears to fix the issue.
In addition, I think we should consider a few other things:
Warn the user that under DEBUG mode, memory used for log messages can grow without bounds (this might be true for other panels already).
Limit the number/age of messages stored (perhaps as a configuration option).