Commit 3c42c5ef authored by Arnaud Fontaine's avatar Arnaud Fontaine

ZODB Components: Resetting Portal Type classes was also clearing Interfaces of...

ZODB Components: Resetting Portal Type classes was also clearing Interfaces of erp5.component.* modules.

__implemented__ and __provides__ attributes are cleared on Portal Type reset
to break a circular reference (11e2b506). Resetting of ZODB Components is
done independently of Portal Type classes, so until the next ZODB Component
reset, Interfaces were not implemented anymore.

11e2b506 states that this creates a memory leak but this is not actually true
as Python garbage collector will break such circular reference and collect
this object, as asserted by a newly introduced Unit Test. This is probably
less efficient than explicitly breaking the circular reference and thus relies
on reference counting (rather than the garbage collector) but doing otherwise
would require introspecting ZODB Components module to find classes implementing
Interfaces (erp5.portal_type only contains classes).

This only takes care of ZODB Components modules but maybe breaking circular
references for Portal Type classes is not even needed (if so, this will be done
in a later commit).
parent 15675094
...@@ -476,7 +476,7 @@ def synchronizeDynamicModules(context, force=False): ...@@ -476,7 +476,7 @@ def synchronizeDynamicModules(context, force=False):
LOG("ERP5Type.dynamic", 0, "Resetting dynamic classes") LOG("ERP5Type.dynamic", 0, "Resetting dynamic classes")
try: try:
for class_name, klass in inspect.getmembers(erp5.portal_type, for _, klass in inspect.getmembers(erp5.portal_type,
inspect.isclass): inspect.isclass):
# Zope Interface is implemented through __implements__, # Zope Interface is implemented through __implements__,
# __implemented__ (both implementedBy instances) and __provides__ # __implemented__ (both implementedBy instances) and __provides__
...@@ -484,11 +484,12 @@ def synchronizeDynamicModules(context, force=False): ...@@ -484,11 +484,12 @@ def synchronizeDynamicModules(context, force=False):
# zope.interface.declarations.implementedByFallback. # zope.interface.declarations.implementedByFallback.
# #
# However both implementedBy and ClassProvides instances keep a # However both implementedBy and ClassProvides instances keep a
# reference to the class itself, thus creating a circular references # reference to the class itself, thus creating a circular references.
# preventing erp5.* classes to be GC even when not being actually used
# anywhere anymore after a reset.
for k in klass.mro(): for k in klass.mro():
if k.__module__.startswith('erp5.'): module_name = k.__module__
if (module_name.startswith('erp5.') and
# Components are reset independently of Portal Types classes
not module_name.startswith('erp5.component.')):
for attr in ('__implements__', '__implemented__', '__provides__'): for attr in ('__implements__', '__implemented__', '__provides__'):
if k.__dict__.get(attr) is not None: if k.__dict__.get(attr) is not None:
delattr(k, attr) delattr(k, attr)
......
...@@ -2599,6 +2599,20 @@ class DifferentFromReference(Person): ...@@ -2599,6 +2599,20 @@ class DifferentFromReference(Person):
that the newly-defined function on ZODB Component can be called as well as that the newly-defined function on ZODB Component can be called as well as
methods from Person Document methods from Person Document
""" """
## Create an Interface assigned to the test ZODB Component to check that
## only resetting Portal Type classes do not have any side-effect on
## Interfaces defined on ZODB Components
from zope.interface import Interface
class ITestPortalType(Interface):
"""Anything"""
def foo():
"""Anything"""
from types import ModuleType
interface_module = ModuleType('ITestPortalType')
interface_module.ITestPortalType = ITestPortalType
import sys
sys.modules['ITestPortalType'] = interface_module
self.failIfModuleImportable('TestPortalType') self.failIfModuleImportable('TestPortalType')
# Create a new Document Component inheriting from Person Document which # Create a new Document Component inheriting from Person Document which
...@@ -2610,17 +2624,27 @@ class DifferentFromReference(Person): ...@@ -2610,17 +2624,27 @@ class DifferentFromReference(Person):
""" """
from erp5.component.document.Person import Person from erp5.component.document.Person import Person
from ITestPortalType import ITestPortalType
import zope.interface
class TestPortalType(Person): class TestPortalType(Person):
def test42(self): def test42(self):
return 42 return 42
""")
zope.interface.implements(ITestPortalType)
def foo(self):
pass
""")
test_component.validate() test_component.validate()
self.tic() self.tic()
# As TestPortalType Document Component has been validated, it should now # As TestPortalType Document Component has been validated, it should now
# be available # be available
self.assertModuleImportable('TestPortalType') self.assertModuleImportable('TestPortalType', reset=False)
self.assertTrue(ITestPortalType.implementedBy(self._module.TestPortalType.TestPortalType))
self._component_tool.reset(force=True,
reset_portal_type_at_transaction_boundary=True)
person_type = self.portal.portal_types.Person person_type = self.portal.portal_types.Person
person_type_class = person_type.getTypeClass() person_type_class = person_type.getTypeClass()
...@@ -2636,19 +2660,12 @@ class TestPortalType(Person): ...@@ -2636,19 +2660,12 @@ class TestPortalType(Person):
# assigned to a Person # assigned to a Person
self.failIfHasAttribute(person, 'test42') self.failIfHasAttribute(person, 'test42')
self.failIfHasAttribute(self._module, 'TestPortalType') self.failIfHasAttribute(self._module, 'TestPortalType')
self.assertFalse(ITestPortalType.providedBy(person))
self.assertFalse(ITestPortalType.implementedBy(person.__class__))
for klass in person.__class__.mro(): for klass in person.__class__.mro():
self.assertNotEqual(klass.__name__, 'TestPortalType') self.assertNotEqual(klass.__name__, 'TestPortalType')
# Reset Portal Type classes to ghost to make sure that everything is reset def _check():
self._component_tool.reset(force=True,
reset_portal_type_at_transaction_boundary=True)
# TestPortalType must be available in type class list
self.assertTrue('TestPortalType' in person_type.getDocumentTypeList())
try:
person_type.setTypeClass('TestPortalType')
self.commit()
self.assertHasAttribute(person, 'test42') self.assertHasAttribute(person, 'test42')
self.assertEqual(person.test42(), 42) self.assertEqual(person.test42(), 42)
...@@ -2658,6 +2675,21 @@ class TestPortalType(Person): ...@@ -2658,6 +2675,21 @@ class TestPortalType(Person):
self.assertTrue(self._module.TestPortalType.TestPortalType in person.__class__.mro()) self.assertTrue(self._module.TestPortalType.TestPortalType in person.__class__.mro())
from erp5.component.document.Person import Person as PersonDocument from erp5.component.document.Person import Person as PersonDocument
self.assertTrue(PersonDocument in person.__class__.mro()) self.assertTrue(PersonDocument in person.__class__.mro())
self.assertTrue(ITestPortalType.providedBy(person))
self.assertTrue(ITestPortalType.implementedBy(person.__class__))
# Reset Portal Type classes to ghost to make sure that everything is reset
self._component_tool.reset(force=True,
reset_portal_type_at_transaction_boundary=False)
# TestPortalType must be available in type class list
self.assertTrue('TestPortalType' in person_type.getDocumentTypeList())
try:
person_type.setTypeClass('TestPortalType')
self.commit()
_check()
self.portal.portal_types.resetDynamicDocuments()
_check()
finally: finally:
person_type.setTypeClass('Person') person_type.setTypeClass('Person')
...@@ -2693,6 +2725,101 @@ class TestWithImport(TestImported): ...@@ -2693,6 +2725,101 @@ class TestWithImport(TestImported):
self.assertModuleImportable('TestWithImport') self.assertModuleImportable('TestWithImport')
self.assertModuleImportable('TestImported') self.assertModuleImportable('TestImported')
def testGC(self):
"""
Zope Implements and ClassProvides keep a reference to the class itself,
thus creating a circular reference which can only be garbage collected by
'gc' module (and not by reference counting).
So check that ZODB Components modules are properly garbage collectable
after a reset (in 'gc' terms: considered 'unreachable' but 'collectable'
and could be freed).
"""
from zope.interface import Interface
class ITestGC(Interface):
"""Anything"""
def foo():
"""Anything"""
from types import ModuleType
interface_module = ModuleType('ITestGC')
interface_module.ITestGC = ITestGC
import sys
sys.modules['ITestGC'] = interface_module
self.failIfModuleImportable('TestGC')
test_component = self._newComponent(
'TestGC',
"""from Products.ERP5Type.XMLObject import XMLObject
from ITestGC import ITestGC
import zope.interface
class TestGC(XMLObject):
zope.interface.implements(ITestGC)
def foo(self):
pass
""")
self.tic()
self.failIfModuleImportable('TestGC')
test_component.validate()
self.tic()
import gc
initial_gc_debug_flags = gc.get_debug()
initial_stderr = sys.stderr
try:
gc.disable()
gc.collect()
self.assertModuleImportable('TestGC', reset=False)
class_id = id(self._module.TestGC.TestGC)
Implements_id = id(self._module.TestGC.TestGC.__implemented__)
ClassProvides_id = id(self._module.TestGC.TestGC.__provides__)
self.assertEqual(gc.collect(), 0)
self.assertEqual(gc.garbage, [])
self._component_tool.reset(force=True)
gc.collect()
self.assertEqual(gc.garbage, [])
import sys
from cStringIO import StringIO
import erp5.component
gc.set_debug(
gc.DEBUG_STATS |
gc.DEBUG_UNCOLLECTABLE |
gc.DEBUG_COLLECTABLE |
gc.DEBUG_OBJECTS |
gc.DEBUG_INSTANCES)
stderr = StringIO()
sys.stderr = stderr
# Still not garbage collectable as RefManager still keeps a reference
erp5.component.ref_manager.clear()
# Once deleted, the ZODB Component module must be collectable...
self.assertNotEqual(gc.collect(), 0)
finally:
gc.set_debug(initial_gc_debug_flags)
gc.enable()
sys.stderr = initial_stderr
# And make sure that it has really be collected thanks to DEBUG_COLLECTABLE
self.assertEqual(gc.garbage, [])
stderr.seek(0)
found_line_list = []
for line in stderr:
if ('0x%x>' % class_id in line or
'0x%x>' % Implements_id in line or
'0x%x>' % ClassProvides_id in line):
found_line_list.append(line)
self.assertEqual(
['gc: collectable <ClassProvides 0x%x>\n' % ClassProvides_id,
'gc: collectable <ExtensionClass.ExtensionClass 0x%x>\n' % class_id,
'gc: collectable <Implements 0x%x>\n' % Implements_id],
sorted(found_line_list))
from Products.ERP5Type.Core.TestComponent import TestComponent from Products.ERP5Type.Core.TestComponent import TestComponent
class TestZodbTestComponent(_TestZodbComponent): class TestZodbTestComponent(_TestZodbComponent):
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment