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 )

Reply via email to