Object leak on reference flush

Bug #475148 reported by Thomas Herve
14
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Launchpad itself
Fix Released
High
Данило Шеган
Storm
Fix Released
High
Thomas Herve

Bug Description

In some circumstances, object infos attached to another object infos via a reference are leaked through the "changed" event, as the "flushed" event is not registered. Adding a hook for "flushed" seems to fix the problem, but I don't know exactly when this problem actually appears.

Related branches

Revision history for this message
Stuart Bishop (stub) wrote :

Objects with foreign key references register for changed events. If you flush these objects and let them disappear from scope and all the caches, they are garbage collected but leave their ObjectInfo records still attached and subscribed to the change events.

Revision history for this message
Stuart Bishop (stub) wrote :

Targetting for 0.16 since we have a patch and this is making bits of Launchpad explode.

Changed in storm:
importance: Undecided → High
milestone: none → 0.16
status: New → Confirmed
Changed in rosetta:
status: New → Confirmed
importance: Undecided → High
Revision history for this message
Stuart Bishop (stub) wrote :

Assigning a task to rosetta as this bug is blocking message sharing data migration. We can wait for 0.16 or put together a package with the fix to use in the interim.

Revision history for this message
Данило Шеган (danilo) wrote :

If we are having a re-rollout, this is something I'd like to see in it.

Changed in rosetta:
status: Confirmed → Triaged
Thomas Herve (therve)
Changed in storm:
assignee: nobody → Thomas Herve (therve)
summary: - ObjectInfo leak on reference flush
+ Object leak on reference flush
Revision history for this message
James Henstridge (jamesh) wrote :

While breaking the cached reference when a flush occurs might solve this bug, do we really want to invalidate this cache at that point?

Given we've had a number of problems with EventSystem holding references longer than we'd like, might it be worth extending it to use weakrefs to remove this class of problem completely?

Django's signal system behaves this way (the main complication is that bound instance method objects are flyweight, so it needs to reference the instance object and unbound instance method).

Revision history for this message
Thomas Herve (therve) wrote :

Merged in storm trunk in r342.

Changed in storm:
status: Confirmed → Fix Committed
Revision history for this message
Данило Шеган (danilo) wrote :

With the time to test this slipping, we are going to QA this for 3.1.11 LP release due next week, but only patched with this revision: we can't wait for a new release and live with only few days of QA today.

Changed in rosetta:
milestone: none → 3.1.11
Revision history for this message
Данило Шеган (danilo) wrote :

FYI, we have QAd this fix and it does indeed fix the leak we've experienced with our script.

Changed in rosetta:
assignee: nobody → Данило Шеган (danilo)
status: Triaged → In Progress
Revision history for this message
Данило Шеган (danilo) wrote :

New storm integrated into lp:launchpad/devel r9949.

Revision history for this message
Ursula Junque (ursinha) wrote : A commit mentioned this bug
Changed in rosetta:
status: In Progress → Fix Committed
tags: added: qa-needstesting
Jamu Kakar (jkakar)
Changed in storm:
status: Fix Committed → Fix Released
tags: added: qa-ok
removed: qa-needstesting
Changed in rosetta:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.