sentry-python icon indicating copy to clipboard operation
sentry-python copied to clipboard

Memory leak during exception logging

Open vgrytsun opened this issue 1 year ago • 8 comments

When using logging.exception sentry does not release exception instance, which results in the memory leak. Exception could be pretty heavy, so depending on the app, it could even cause an OOM.

import logging
import weakref
import time
import sentry_sdk

weak_exception = None
class CustomException(Exception):
    pass

def callback(ref):
    print("Exception was garbage collected!")

sentry_sdk.init(<init params>)
try:
    raise CustomException('Custom Exception')
except Exception as e:
    logging.exception("Exception logging")
    weak_exception = weakref.ref(e, callback)
    sentry_sdk.flush()

while True:
    time.sleep(1)
    print(weak_exception())

In the code above, with sentry OFF, exception is garbage collected almost immediately, with sentry ON it stays around even after sentry is forced to flush all the events.

It seems like pretty severe issue, so curious if there is any workaround.

Version 2.5

vgrytsun avatar Jun 13 '24 05:06 vgrytsun

Hey @vgrytsun, thanks for bringing this up. The open reference is held by the DedupeIntegration here. The integration remembers the last thing that happened so that it can filter out any duplicates coming in afterwards. It always just holds the last exception, so as soon as you raise and capture another exception, the first CustomException gets garbage collected (while the new exception is kept around instead). It shouldn't accumulate over time.

sentrivana avatar Jun 13 '24 13:06 sentrivana

@sentrivana thank you for a quick response! So the workaround is to disable DedupeIntegration (the only downside i can see is that each client will be reporting more events) ? Also, i am curious whether instead of last exception, sentry should hold a reference to the last event, which should be much lighter object (it's pretty much nested dictionary of strings)

vgrytsun avatar Jun 13 '24 16:06 vgrytsun

@vgrytsun You could disable the DedupeIntegration, yes, with exactly the downside you mentioned; you might be getting duplicate events.

TBH I'm not sure if the event is in general a lighter object than an exception, my gut feeling is that it's usually the other way around. What kind of exceptions are you seeing that are so big?

sentrivana avatar Jun 17 '24 12:06 sentrivana

Exception has a reference to traceback instance, which holds stack frames and their local variables. And those local variables could be arbitrary sizes depending on the application. So for the memory intensive app it could cause perf degradation or out of memory crashes.

vgrytsun avatar Jun 17 '24 17:06 vgrytsun

We can definitely investigate whether we need to keep the whole exception or if the event or just some part of the exception would be enough. 👍🏻

sentrivana avatar Jun 18 '24 14:06 sentrivana

This issue has been reported again in #4327

szokeasaurusrex avatar May 05 '25 08:05 szokeasaurusrex

Hey, we've also encountered this leak, in our case that was because Sentry.init was called in our master process, so _last_seen ContextVar were put in the app context, not request (we have FastAPI app). That led to pretty big leaks on capturing many small and similar ValueError's.

~~We've managed to fix that by using DedupeIntegration.reset_last_seen() at the end of each request, since it's ok for us when same errors in different request considered unique.~~

UPDATE: nevermind, I was re-testing this and found that I somehow commented DedupeIntegration, and that's why it worked, reset_last_seen didn't help =(

Hope this helps, have a great day =)

gooverdian avatar Jun 03 '25 19:06 gooverdian

Thanks for the info @gooverdian We still do not have a final solution for this, but I am trying things...

antonpirker avatar Jun 05 '25 11:06 antonpirker