DEV Community

Cover image for A maintainer, me, and Sentry's own AI all reached for the same wrong fix
JonathanSolvesProblems
JonathanSolvesProblems

Posted on

A maintainer, me, and Sentry's own AI all reached for the same wrong fix

Summer Bug Smash: Smash Stories 🐛🛹

This is a submission for DEV's Summer Bug Smash: Smash Stories powered by Sentry.

Three people tried to fix the same memory leak in Sentry's Python SDK. One was a Sentry maintainer. One was me. One was Sentry's own AI debugger.

All three fixes were wrong, and they were wrong for the same underlying reason. That convergence is the actual story, and it took me a while to see it, because I was too busy being pleased with my own version.

The leak, briefly

DedupeIntegration remembers the last exception it saw so it can drop a duplicate report. It stores a weak reference, because holding the exception would hold its traceback, and a traceback holds every frame, and a frame holds every local variable in it.

Then there is this:

# we can only weakref non builtin types
try:
    integration._last_seen.set(weakref.ref(exc))
except TypeError:
    integration._last_seen.set(exc)
Enter fullscreen mode Exit fullscreen mode

You cannot weak-reference a ValueError. So the fallback runs, and it does the exact thing the weak reference existed to prevent. It lives in a ContextVar, so under asyncio every task keeps its own copy: one retained exception, one traceback, one full set of frame locals, per live task.

Measured across 200 long-lived sessions holding a megabyte each: 205 MB retained, all 200 still reachable after gc.collect().

That part I have written up in full elsewhere. This post is about the three fixes.

Attempt one: the maintainer

Digging through the fork's branch list, I found two abandoned branches from September 2025, neither merged. The first replaces the stored exception with a SHA-256 fingerprint over (type_module, type_name, id(exc_value)).

It is a reasonable instinct. Store a cheap value, not the object. The problem is id().

Not retaining the exception is precisely what frees its address, and CPython reuses freed addresses immediately. Over 2000 distinct, sequentially allocated ValueErrors:

distinct exceptions created : 2000
distinct fingerprints       : 2
address-reuse collisions    : 1998
Enter fullscreen mode Exit fullscreen mode

Two fingerprints for two thousand distinct errors. Every collision is a real error that gets silently dropped as a duplicate. The fix trades a memory leak for losing almost every error you report, which is worse than the leak.

Attempt two: me

I did not see that branch until much later, which is lucky, because I made the same class of mistake independently.

My idea was a value fingerprint with no id() in it: exception type, message, and the origin frame from the traceback. All immutable primitives. Nothing retained. I was confident enough to post it publicly on the issue.

The SDK's own test suite killed it in two different ways within about a minute of running.

test_breadcrumbs captures two distinct, never-raised ValueError() instances. No traceback, no message, so both fingerprints are identical and the second event is dropped.

test_option_before_breadcrumb is worse. It calls the same function three times, each raising a separate ValueError("aha!") from the same line. Same type, same message, same origin frame. Three identical fingerprints, two events wrongly deduplicated.

The flaw is not in my choice of fields. It is structural:

A value fingerprint cannot distinguish "the same exception object captured twice" from "the same error raised twice from the same line."

The first must deduplicate. The second must not. By value, they are identical. No amount of extra fields fixes that, because the thing being asked for is not a property of the value.

I went back to the issue and said I had been wrong.

Attempt three: the AI

Later I pointed Sentry's own AI debugger, Seer, at the issue in my own Sentry project.

Its diagnosis was genuinely excellent, and better than I expected. It reached past my application code into a third-party SDK, identified the ContextVar strong reference, identified that weakref.ref raises TypeError on builtin exception types, and identified that ContextVars are per-task under asyncio. That last detail had cost me a full debugging round to work out on my own. It cited dedupe.py lines as evidence, so it had gone and read the source rather than guessing from a stack trace.

Then it proposed the fix:

store a hashable identity tuple like (type(exc), id(exc))

id() again. I ran its exact tuple:

distinct errors raised         : 2000
events wrongly dropped as dupe : 1999  (100.0%)
Enter fullscreen mode Exit fullscreen mode

Three independent parties. Two humans and a model. Two different wrong answers that are secretly the same wrong answer.

The thing all three of us did

Every one of us tried to represent identity with a value.

An object's identity is not a property you can read off it. It is the fact of the object existing, distinct from every other object, for as long as it lives. A fingerprint made of type and message describes what the exception is like. An address describes where it currently sits. Neither survives the thing that makes identity useful, which is that two objects that look the same are still two objects.

Once I could say that sentence, the fix was obvious, and it is not a fingerprint at all:

class _DedupeToken:
    __slots__ = ("__weakref__",)
Enter fullscreen mode Exit fullscreen mode

Attach one token to the exception and weakly reference the token. There is exactly one per exception object and it lives exactly as long as the exception does, so comparing tokens is comparing identity. The ContextVar holds nothing that keeps a traceback alive, and deduplication behaviour does not change at all.

Same 200 sessions, after the fix: 0.7 MB, nothing pinned.

The part I did not expect

There is a coda that I only found while going back through a screen recording, and it is the most interesting thing I learned all week.

Seer did not only propose a plan. It also opened a pull request with generated code. The code is not what the plan said.

try:
    integration._last_seen.set(weakref.ref(exc))
except TypeError:
    pass
Enter fullscreen mode Exit fullscreen mode

No identity tuple anywhere. It simply stops storing for builtin exceptions. That does remove the leak, and it has none of the address-reuse problem, but it silently disables deduplication for the exception types most errors actually are. It is also, precisely, the fix the maintainers had already declined on the issue months earlier.

So the plan and the patch disagree with each other, and they are wrong in two different ways.

I want to be fair here, because the diagnosis was the hard part and it got that right, from evidence, without my framing. But if you let an agent open pull requests: read the diff, not the summary. They are not required to match, and here they did not.

What I took away

The comment # we can only weakref non builtin types sat directly above the line that caused the leak, for years. It described the hazard accurately and then the next line walked into it. A fallback is still a code path, and this one ran for the overwhelming majority of real exceptions.

And the more useful lesson: when several competent people independently produce the same wrong answer, that is information. It usually means the obvious framing of the problem is the thing that is wrong, not the people. Three of us reached for a fingerprint because "store something small instead of the object" is a good habit. The habit was fine. The question was wrong.

Everything here is reproducible, including all three failed fixes, at JonathanSolvesProblems/sentry-dedupe-leak-repro. id_reuse_hazard.py runs the maintainer's approach and Seer's, side by side, and prints exactly how many errors each one would lose.

Top comments (0)