#149 — Quills memory leaks at the point of WeblogEntryCatalogBrain usage.

by Volodymyr Cherepanyak — last modified Feb 25, 2009 07:49 AM
State Resolved
Version: 1.6
Area Functionality
Issue type Bug
Severity Medium
Submitted by Volodymyr Cherepanyak
Submitted on Nov 04, 2008
Responsible Jan Hackel
Target release: 1.7
Return to tracker
While evaluating Quills 1.6 (Beta release) in Plone 3.1.6 we at Quintagroup found that it leads to memory leaks.

The WeblogEntryCatalogBrain is used in Weblog.getEntries() function and few other places. Our investigation showed that the increment of Refcounts happens around its usage.

The WeblogEntryCatalogBrain was brought by tim2p from /branches/maurits-traversal with [43913] checkin: http://dev.plone.org/collective/changeset/43913
Steps to reproduce:
* Create Weblog add a few entries
* Open the /Control_Panel/DebugInfo
* Refresh the Weblog front page few times
* Track the number of next classes:
    - Products.ZCatalog.CatalogBrains.AbstractCatalogBrain (+222)
    - uills.app.weblogentrybrain.WeblogEntryCatalogBrain (+148)
    - quills.app.utilities.QuillsMixin (+74)

the numbers we actually seen are in parenthesis. (These numbers never go down)
Added by Jan Hackel on Nov 05, 2008 10:09 PM
Issue state: unconfirmed → open
Target release: None → 1.7
Responsible manager: (UNASSIGNED) → jhackel
Confirmed.
Added by Jan Hackel on Nov 07, 2008 02:09 PM
Issue state: open → in-progress
It seems this is caused by quills.app.utilities.QuillsMix being an old style class. Letting it inherit from object makes this problem apparently go away. I am searching for clues on why that is so, yet.

Please verify with the aforementioned fix. It might be useful to have an eye on AbstractCatalogBrain now.
Added by Jan Hackel on Nov 17, 2008 01:04 PM
The fix is now in the repository. There is no test-case, though.

See http://dev.plone.org/collec[…]s.app%2Ftrunk&new=75991
Added by Tim Hicks on Jan 09, 2009 12:19 PM
Can this issue be closed now, then?
Added by Volodymyr Cherepanyak on Jan 09, 2009 12:38 PM
Inheritance of QuillsMixin from Object resolves only one third of the memory leaks.

We analyzed the Catalog.useBrains() method, each time it is called new value of mybrains class is saved into volatile attribute causing the leak. Though it looks like over usage of the useBrains() is problem. It would be good to call the useBrains() only once when Quills is installed. But any other product can call useBrains() too.

So, we implemented Catalog monkey patch product to fix the issue: http://projects.quintagroup.com/[…]/trunk
(the actual patch is http://projects.quintagroup.com/[…]/__init__.py).

In the patch we cache all brains that were posted to useBrains() in a list and reuse the classes from this cache when appropriate.
Added by Tim Hicks on Jan 17, 2009 07:08 PM
Hmm, so are you saying that this is a bug in Products.ZCatalog? If so, shouldn't we report/fix it there?
Added by Volodymyr Cherepanyak on Jan 17, 2009 08:09 PM
We debugged and tried to fix symptoms. But we can not say what is the root of issue bug in Catalog or miss use of Catalog.useBrains().

We should consult with ZCatalog creators/maintainers regarding the usage of Catalog.useBrains() method. As of the moment we didn't find good documentation and usecases for the function (and even other good examples of its usage).
Added by Tim Hicks on Jan 17, 2009 10:11 PM
This is a bit of a stab in the dark, but could the problem be that quills.app.weblogentrybrain.WeblogEntryCatalogBrain inherits (via quills.app.utilities.QuillsMixin) from `object' - making it a new-style class? Does this cause problems with acquisition and/or persistence that leads to the memory leak? Just a thought.
Added by Volodymyr Cherepanyak on Jan 17, 2009 10:54 PM
Inheriting the quills.app.weblogentrybrain.WeblogEntryCatalogBrain from class "object" cuts only 1/3 of the leak. Other 2/3 of leak happens on each call of Catalog.useBrains when volatile attributes are set (self._v_brains = brains; self._v_result_class = mybrains).

We tried numerous tricks with WeblogEntryCatalogBrain and WeblogMixin but still the number of Products.ZCatalog.CatalogBrains.AbstractCatalogBrain grow in memory (as the useBrains() is called each time when Quills render some listing). Thus I see the only option, we need to find answers to the next questions "How the Catalog.useBrains() function is supposed to be used?" and "Does the Catalog.useBrains() has bug or does Quills use it incorrectly?"

Added by Tim Hicks on Jan 17, 2009 11:44 PM
Ah, sorry, I just re-read the thread and saw that you actually swapped to inheriting from `object' on purpose :). Are you in touch with the ZCatalog maintainer(s)/dev-list?
Added by Volodymyr Cherepanyak on Jan 18, 2009 10:19 AM
zope-dev list is definitely the place to get answers.
Added by Jan Hackel on Feb 14, 2009 09:19 PM
It seems, that in the course of fixing issue #162, we will stop using "useBrains" anyway. So this eventually will fix this issue too.
Added by Jan Hackel on Feb 24, 2009 10:47 PM
Issue state: In progress → Resolved
There is now a fix at rev 81282 (http://dev.plone.org/collective/changeset/81282). It is a rather excessive change, so there might be new bugs included. Crossing fingers, there are not...

By the way, Volodymyr, did you publish somewhere how you debugged this memory issue? It would be quite educational, I guess. I certainly would like to learn more about it!

Please confirm if this fix actually mends this issue.
Added by Volodymyr Cherepanyak on Feb 25, 2009 07:49 AM
We followed DebugInfo numbers in control panel as described in the ticket "Steps to reproduce" section. Then involved pdb to see which function calls raise the numbers. If you feel it is to terse description let me know I will come back with more details.

No responses can be added.