Re: [Zope-dev] AccessControl C extension, ExtensionBase, Cylic memory and tp_traverse
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 11/07/2012 08:27 AM, Sylvain Viollon wrote: Op 7 nov 2012, om 14:21 heeft Sylvain Viollon het volgende geschreven: Op 6 nov 2012, om 20:17 heeft Leonardo Rochael Almeida het volgende geschreven: Hi Silvain, Thanks for the deep analysis. Did you put this code in a branch somewhere for us to take a look? I created a branch here: http://svn.zope.org/AccessControl/branches/sylvain-memory-leak/ Hum, looking at it though the web, I didn't see the code was indented with tabs (?!?!?!). My emacs re-indented some lines in a kind of funky way, so this messed up the diff. Sorry for that. I can later reedit the file if needed. Can you please clean up the diff so that the real changes can be reviewed? At the moment, they are lost in the noise. Tres. - -- === Tres Seaver +1 540-429-0999 tsea...@palladion.com Palladion Software Excellence by Designhttp://palladion.com -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://www.enigmail.net/ iEYEARECAAYFAlCb0KAACgkQ+gerLs4ltQ4+TwCbBirtD2CvPZbshPwbFC5ffiDL PXcAnRX2lZAElvGQz0O4oFOL7reeB1YL =enZb -END PGP SIGNATURE- ___ 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 )
Re: [Zope-dev] AccessControl C extension, ExtensionBase, Cylic memory and tp_traverse
Op 6 nov 2012, om 20:17 heeft Leonardo Rochael Almeida het volgende geschreven: Hi Silvain, Thanks for the deep analysis. Did you put this code in a branch somewhere for us to take a look? I created a branch here: http://svn.zope.org/AccessControl/branches/sylvain-memory-leak/ 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 )
Re: [Zope-dev] AccessControl C extension, ExtensionBase, Cylic memory and tp_traverse
Op 7 nov 2012, om 14:21 heeft Sylvain Viollon het volgende geschreven: Op 6 nov 2012, om 20:17 heeft Leonardo Rochael Almeida het volgende geschreven: Hi Silvain, Thanks for the deep analysis. Did you put this code in a branch somewhere for us to take a look? I created a branch here: http://svn.zope.org/AccessControl/branches/sylvain-memory-leak/ Hum, looking at it though the web, I didn't see the code was indented with tabs (?!?!?!). My emacs re-indented some lines in a kind of funky way, so this messed up the diff. Sorry for that. I can later reedit the file if needed. 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] AccessControl C extension, ExtensionBase, Cylic memory and tp_traverse
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 )
Re: [Zope-dev] AccessControl C extension, ExtensionBase, Cylic memory and tp_traverse
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 )
