From fcd75f48986d6b7561b837c35d29a00fbd6d472d Mon Sep 17 00:00:00 2001 From: unknown Date: Thu, 7 Jun 2012 16:17:06 +0200 Subject: [PATCH] * Fix about memory issue that causes multiple mediator instanciations for a unique view. * Added some unit tests to show the issue and the utility of the fix. Those unit tests might be a bit random, cause of the unpredicable behavior of the GC. --- .../modular/base/ModuleMediatorMap.as | 25 +++++ .../utilities/modular/base/ModuleViewMap.as | 23 +++++ .../modular/mvcs/FixedModuleContext.as | 42 +++++++++ src_test/LaunchTestSuite.as | 31 ++++++ .../modular/mvcs/ModuleContextTest.as | 94 +++++++++++++++++++ src_test/testutils/TestStage.as | 14 +++ .../testutils/modules/FixedModuleAContext.as | 37 ++++++++ src_test/testutils/modules/ModuleAContext.as | 37 ++++++++ src_test/testutils/modules/ModuleAMediator.as | 17 ++++ src_test/testutils/modules/ModuleAView.as | 14 +++ 10 files changed, 334 insertions(+) create mode 100644 src/org/robotlegs/utilities/modular/base/ModuleMediatorMap.as create mode 100644 src/org/robotlegs/utilities/modular/base/ModuleViewMap.as create mode 100644 src/org/robotlegs/utilities/modular/mvcs/FixedModuleContext.as create mode 100644 src_test/LaunchTestSuite.as create mode 100644 src_test/org/robotlegs/utilities/modular/mvcs/ModuleContextTest.as create mode 100644 src_test/testutils/TestStage.as create mode 100644 src_test/testutils/modules/FixedModuleAContext.as create mode 100644 src_test/testutils/modules/ModuleAContext.as create mode 100644 src_test/testutils/modules/ModuleAMediator.as create mode 100644 src_test/testutils/modules/ModuleAView.as diff --git a/src/org/robotlegs/utilities/modular/base/ModuleMediatorMap.as b/src/org/robotlegs/utilities/modular/base/ModuleMediatorMap.as new file mode 100644 index 0000000..a3dfb8a --- /dev/null +++ b/src/org/robotlegs/utilities/modular/base/ModuleMediatorMap.as @@ -0,0 +1,25 @@ +package org.robotlegs.utilities.modular.base +{ +import org.robotlegs.base.MediatorMap; +import org.robotlegs.core.IInjector; +import org.robotlegs.core.IReflector; + +import flash.display.DisplayObjectContainer; + +/** + * @author dlaurent 7 juin 2012 + */ +public class ModuleMediatorMap extends MediatorMap +{ + public function ModuleMediatorMap( contextView : DisplayObjectContainer, injector : IInjector, + reflector : IReflector ) + { + super( contextView, injector, reflector ); + } + + public function dispose() : void + { + removeListeners(); + } +} +} diff --git a/src/org/robotlegs/utilities/modular/base/ModuleViewMap.as b/src/org/robotlegs/utilities/modular/base/ModuleViewMap.as new file mode 100644 index 0000000..5724002 --- /dev/null +++ b/src/org/robotlegs/utilities/modular/base/ModuleViewMap.as @@ -0,0 +1,23 @@ +package org.robotlegs.utilities.modular.base +{ +import org.robotlegs.base.ViewMap; +import org.robotlegs.core.IInjector; + +import flash.display.DisplayObjectContainer; + +/** + * @author dlaurent 7 juin 2012 + */ +public class ModuleViewMap extends ViewMap +{ + public function ModuleViewMap( contextView : DisplayObjectContainer, injector : IInjector ) + { + super( contextView, injector ); + } + + public function dispose() : void + { + removeListeners(); + } +} +} diff --git a/src/org/robotlegs/utilities/modular/mvcs/FixedModuleContext.as b/src/org/robotlegs/utilities/modular/mvcs/FixedModuleContext.as new file mode 100644 index 0000000..8963965 --- /dev/null +++ b/src/org/robotlegs/utilities/modular/mvcs/FixedModuleContext.as @@ -0,0 +1,42 @@ +package org.robotlegs.utilities.modular.mvcs +{ +import org.robotlegs.core.IInjector; +import org.robotlegs.core.IMediatorMap; +import org.robotlegs.core.IViewMap; +import org.robotlegs.utilities.modular.base.ModuleMediatorMap; +import org.robotlegs.utilities.modular.base.ModuleViewMap; + +import flash.display.DisplayObjectContainer; +import flash.system.ApplicationDomain; + +/** + * @author dlaurent 7 juin 2012 + */ +public class FixedModuleContext extends ModuleContext +{ + public function FixedModuleContext( contextView : DisplayObjectContainer = null, autoStartup : Boolean = true, + parentInjector : IInjector = null, applicationDomain : ApplicationDomain = null ) + { + super( contextView, autoStartup, parentInjector, applicationDomain ); + } + + override public function dispose() : void + { + ModuleMediatorMap( _mediatorMap ).dispose(); + ModuleViewMap( _viewMap ).dispose(); + super.dispose(); + } + + override protected function get mediatorMap() : IMediatorMap + { + return _mediatorMap || + ( _mediatorMap = + new ModuleMediatorMap( contextView, injector.createChild( _applicationDomain ), reflector ) ); + } + + override protected function get viewMap() : IViewMap + { + return _viewMap || ( _viewMap = new ModuleViewMap( contextView, injector.createChild( _applicationDomain ) ) ); + } +} +} diff --git a/src_test/LaunchTestSuite.as b/src_test/LaunchTestSuite.as new file mode 100644 index 0000000..e970276 --- /dev/null +++ b/src_test/LaunchTestSuite.as @@ -0,0 +1,31 @@ +package +{ +import testutils.TestStage; + +import org.flexunit.internals.TraceListener; +import org.flexunit.runner.FlexUnitCore; +import org.robotlegs.utilities.modular.mvcs.ModuleContextTest; + +import flash.display.Sprite; + +public class LaunchTestSuite extends Sprite +{ + public function LaunchTestSuite() + { + TestStage.stage = stage; + + var uniqueTestClass : Class = null; + + core = new FlexUnitCore(); + core.addListener( new TraceListener() ); + + if ( uniqueTestClass == null ) + core.run( ModuleContextTest ); + else + core.run( uniqueTestClass ); + } + + private var core : FlexUnitCore; +} +} + diff --git a/src_test/org/robotlegs/utilities/modular/mvcs/ModuleContextTest.as b/src_test/org/robotlegs/utilities/modular/mvcs/ModuleContextTest.as new file mode 100644 index 0000000..a9a061e --- /dev/null +++ b/src_test/org/robotlegs/utilities/modular/mvcs/ModuleContextTest.as @@ -0,0 +1,94 @@ +package org.robotlegs.utilities.modular.mvcs +{ +import testutils.TestStage; +import testutils.modules.FixedModuleAContext; +import testutils.modules.ModuleAContext; +import testutils.modules.ModuleAMediator; + +import org.flexunit.asserts.assertEquals; +import org.flexunit.asserts.assertFalse; +import org.flexunit.asserts.assertTrue; +import org.flexunit.async.Async; + +import flash.events.Event; +import flash.events.TimerEvent; +import flash.system.System; +import flash.utils.Timer; + +/** + * @author dlaurent 7 juin 2012 + */ +public class ModuleContextTest +{ + + [Before( async )] + public function setup() : void + { + System.gc(); + ModuleAMediator.numInstances = 0; + + // This strange thing is intended to let the time for the GC to clean memory (sometimes + // it is a bit more longer and listeners from the first test are still there when running the second test). + var delayer : Timer = new Timer( 2000, 1 ); + Async.proceedOnEvent( this, delayer, TimerEvent.TIMER_COMPLETE, 3000 ); + delayer.start(); + } + + /** + * With the original ModuleContext, we see that the ADDED_TO_STAGE listeners (viewMap and mediatorMap) + * are not unregistered if the GC didn't pass after the dispose of moduleAContext. + * + * NOTE : This test is a bit random, it can fail if the GC does the garbage collection just after the + * "moduleAContext.dispose()" instruction. + */ + [Test] + public function after_ModuleContext_shutdown__contextView_still_have_ADDED_TO_STAGE_listener() : void + { + // --o Setup + var moduleAContext : ModuleAContext = new ModuleAContext( TestStage.stage, false ); + moduleAContext.startup(); + // --o Exercise + moduleAContext.dispose(); + // --o Verify + assertTrue( TestStage.stage.hasEventListener( Event.ADDED_TO_STAGE ) ); + } + + /** + * With the fix, the ADDED_TO_STAGE listeners (viewMap and mediatorMap) are released. + */ + [Test] + public function after_FixedModuleContext_shutdown__contextView_shouldnt_have_ADDED_TO_STAGE_listener() : void + { + // --o Setup + var moduleAContext : FixedModuleAContext = new FixedModuleAContext( TestStage.stage, false ); + moduleAContext.startup(); + // --o Exercise + moduleAContext.dispose(); + // --o Verify + assertFalse( TestStage.stage.hasEventListener( Event.ADDED_TO_STAGE ) ); + } + + /** + * Here, we can see that the memory leak issue has the consequence to instanciate too much mediators + * for a unique view instance. + * + * NOTE : This test is a bit random, it can fail if the GC does the garbage collection just after the + * "moduleAContext.dispose()" instruction. + */ + [Test] + public function with_memory_leak_issue__too_much_mediators_are_created_after_second_instanciation() : void + { + // --o Setup + var moduleAContext : ModuleAContext = new ModuleAContext( TestStage.stage, false ); + moduleAContext.startup(); + moduleAContext.dispose(); + // --o Exercise + assertEquals( 1, ModuleAMediator.numInstances ); + // --o Verify + moduleAContext = new ModuleAContext( TestStage.stage, false ); + moduleAContext.startup(); + // Two mediators have been created for this single module startup. + assertEquals( 3, ModuleAMediator.numInstances ); + } +} +} diff --git a/src_test/testutils/TestStage.as b/src_test/testutils/TestStage.as new file mode 100644 index 0000000..07a1165 --- /dev/null +++ b/src_test/testutils/TestStage.as @@ -0,0 +1,14 @@ +package testutils +{ + +import flash.display.Stage; + +/** + * Used to access stage while running unit tests. + * @author dlaurent 7 juin 2012 + */ +public class TestStage +{ + public static var stage : Stage; +} +} diff --git a/src_test/testutils/modules/FixedModuleAContext.as b/src_test/testutils/modules/FixedModuleAContext.as new file mode 100644 index 0000000..bc3d6ab --- /dev/null +++ b/src_test/testutils/modules/FixedModuleAContext.as @@ -0,0 +1,37 @@ +package testutils.modules +{ +import org.robotlegs.core.IInjector; +import org.robotlegs.utilities.modular.mvcs.FixedModuleContext; + +import flash.display.DisplayObjectContainer; +import flash.system.ApplicationDomain; + +/** + * @author dlaurent 7 juin 2012 + */ +public class FixedModuleAContext extends FixedModuleContext +{ + public function FixedModuleAContext( contextView : DisplayObjectContainer = null, autoStartup : Boolean = true, + parentInjector : IInjector = null, applicationDomain : ApplicationDomain = null ) + { + super( contextView, autoStartup, parentInjector, applicationDomain ); + } + + override public function startup() : void + { + injector.mapSingleton( ModuleAView ); + mediatorMap.mapView( ModuleAView, ModuleAMediator ); + + contextView.addChild( injector.getInstance( ModuleAView ) ); + + super.startup(); + } + + override public function shutdown() : void + { + contextView.removeChild( injector.getInstance( ModuleAView ) ); + + super.shutdown(); + } +} +} diff --git a/src_test/testutils/modules/ModuleAContext.as b/src_test/testutils/modules/ModuleAContext.as new file mode 100644 index 0000000..343c1b7 --- /dev/null +++ b/src_test/testutils/modules/ModuleAContext.as @@ -0,0 +1,37 @@ +package testutils.modules +{ +import org.robotlegs.core.IInjector; +import org.robotlegs.utilities.modular.mvcs.ModuleContext; + +import flash.display.DisplayObjectContainer; +import flash.system.ApplicationDomain; + +/** + * @author dlaurent 7 juin 2012 + */ +public class ModuleAContext extends ModuleContext +{ + public function ModuleAContext( contextView : DisplayObjectContainer = null, autoStartup : Boolean = true, + parentInjector : IInjector = null, applicationDomain : ApplicationDomain = null ) + { + super( contextView, autoStartup, parentInjector, applicationDomain ); + } + + override public function startup() : void + { + injector.mapSingleton( ModuleAView ); + mediatorMap.mapView( ModuleAView, ModuleAMediator ); + + contextView.addChild( injector.getInstance( ModuleAView ) ); + + super.startup(); + } + + override public function shutdown() : void + { + contextView.removeChild( injector.getInstance( ModuleAView ) ); + + super.shutdown(); + } +} +} diff --git a/src_test/testutils/modules/ModuleAMediator.as b/src_test/testutils/modules/ModuleAMediator.as new file mode 100644 index 0000000..01c663a --- /dev/null +++ b/src_test/testutils/modules/ModuleAMediator.as @@ -0,0 +1,17 @@ +package testutils.modules +{ +import org.robotlegs.utilities.modular.mvcs.ModuleMediator; + +/** + * @author dlaurent 7 juin 2012 + */ +public class ModuleAMediator extends ModuleMediator +{ + public static var numInstances : int = 0; + + public function ModuleAMediator( view : ModuleAView ) + { + numInstances++; + } +} +} diff --git a/src_test/testutils/modules/ModuleAView.as b/src_test/testutils/modules/ModuleAView.as new file mode 100644 index 0000000..3abfe1e --- /dev/null +++ b/src_test/testutils/modules/ModuleAView.as @@ -0,0 +1,14 @@ +package testutils.modules +{ +import flash.display.Sprite; + +/** + * @author dlaurent 7 juin 2012 + */ +public class ModuleAView extends Sprite +{ + public function ModuleAView() + { + } +} +}