Hi Silvain, Thanks for the deep analysis. Did you put this code in a branch somewhere for us to take a look?
Cheers, Leo On Tue, Nov 6, 2012 at 2:54 PM, Sylvain Viollon <sylv...@infrae.com> wrote: > Hello, > > I recently find out I had a memory leak in Silva, with Python 2.7. After > playing a while with various tools (objgraph is nice), I found out I leaked > between two and three objects at every request: > > - an AccessControl.SecurityManagement.SecurityContext instance, > > - two bound methods to ZopeSecurityPolicy.validate and > ZopeSecurityPolicy.checkPermission. > > Of course, those leaks disappears as soon as I use the Python > implementation for the security instead of the one in C. And I have tested it > as well with various older versions of AccessControl, and have the same > behavior. > > Looking closely at the code, I see that those objects are used internally > by the SecurityManager. The SecurityManager is implemented in C, using the > facilities provided by ExtensionClass. The SecurityManager properly implement > the method dealloc, and it is correctly called. However, since it holds > objects managed by the Python garbage collector (namely one called context > that is an instance of AccessControl.SecurityManagement.SecurityContext, one > called policy, that is an instance of ZopeSecurityPolicy, and a two last ones > that are the bound methods ZopeSecurityPolicy.validate and > ZopeSecurityPolicy.checkPermission), it should implements the tp_traverse > method, but it doesn't. This is not done because ExtensionClass uses the slot > for tp_traverse to store something else (namely what goes in tp_methods > later). > > In a local checkout of AccessControl, I converted the SecurityManager C > code not to rely on ExtensionClass anymore, but use the 'new object' C API, > that is mostly equivalent to ExtensionClass, and implemented the tp_traverse > method. This fixed my memory leak. > > I propose you to commit my code in a branch, and after review to merge it > with the branch 2.13 and the trunk of AccessControl. > > I don't see any argument against not using ExtensionClass anymore for > SecurityManager, and it might be possible to convert some other classes to > the 'new object' C API too. I can have a look at it. > > Regards, > > Sylvain, > > -- > Sylvain Viollon -- Infrae > t +31 10 243 7051 -- http://infrae.com > Hoevestraat 10 3033GC Rotterdam -- The Netherlands > > > > _______________________________________________ > Zope-Dev maillist - Zope-Dev@zope.org > https://mail.zope.org/mailman/listinfo/zope-dev > ** No cross posts or HTML encoding! ** > (Related lists - > https://mail.zope.org/mailman/listinfo/zope-announce > https://mail.zope.org/mailman/listinfo/zope ) _______________________________________________ Zope-Dev maillist - Zope-Dev@zope.org https://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - https://mail.zope.org/mailman/listinfo/zope-announce https://mail.zope.org/mailman/listinfo/zope )