robotlegs / robotlegs-framework

An ActionScript 3 application framework for Flash and Flex
https://robotlegs.tenderapp.com/
MIT License
967 stars 261 forks source link

Injector teardown doesn't catch dependency providers #163

Closed bholloway closed 10 years ago

bholloway commented 10 years ago

I'm not sure how relevant this is but I will share as an observation.

Some time ago I was profiling a project to ensure good garbage collection and discovered that the swiftsuspenders IInjector.teardown() does not remove providerMappings.

I've been using an extension (given below) to remove the mappings externally using the SsInternal namespace. However it strikes me that, if this is a issue, something should be done more systematically in the Injector or the Context.

This may only be an issue where the context builder tag is used and is not manually removed. For me it is less natural to remove a declared mxml element.

    public class EnhancedTeardownExtension
    implements IExtension
    {
        use namespace SsInternal;

        /** @inheritDoc */
        public function extend(context:IContext):void
        {
            context.addEventListener(LifecycleEvent.POST_DESTROY, function(event:LifecycleEvent):void {

                // infer the context from the event to remove dependency on extend method
                var context :IContext = event.target as IContext;

                // detach method
                context.removeEventListener(event.type, arguments.callee);

                // remove all the providers
                //  this is an oversight from the injector teardown
                var providers :Dictionary = (context.injector as Injector).providerMappings;
                for (var key:Object in providers) {
                    delete providers[key];
                }
            });
        }

    }
darscan commented 10 years ago

Nice catch. The patch (just the providerMappings loop) should go into the Swiftsuspenders teardown method. I don't think I'll have time for this this w/e, so feel free to write a patch + test and submit a pull request.

After having a quick look at the Swiftsuspenders Injector code I'm not sure that providerMappings really should be a dictionary, as the keys are always strings. A plain object would probably be better.

bholloway commented 10 years ago

My apologies for not getting to this earlier.

Having checkout out Swiftsuspenders I see (unsurprisingly) someone has beat me to it.

This appears to now be fixed on Swiftsuspenders master as of 12 hours ago. So this issue should be solved upon next integration of Swiftsuspenders SWC into robotlegs/framework. Good job vaukalak!

Refer Injector.as:495. https://github.com/robotlegs/swiftsuspenders/commit/16f2ab2b42baa18af1b9370e95990dc69b889087

Now that I see how relatively simple the whole process is I hope to do better next time.

darscan commented 10 years ago

Hiya. No problem. This should now be fixed in RL 2.2.1.