tschneidereit / SwiftSuspenders

NOTE: Find the offical repo at http://github.com/robotlegs/swiftsuspenders
https://github.com/robotlegs/swiftsuspenders
MIT License
280 stars 89 forks source link

Map Singleton bug #48

Open soundstep opened 13 years ago

soundstep commented 13 years ago

Hi, I think I found a bug in a very specific case. The getInstance will create a new instance in a PostConstruct handler, even if it is mapped as singleton.

My problem was in a more complicated scenario of an command that is dispatched from a PostConstruct in ClassA, and this ClassA was injected in the command class for monitoring purpose, throwing an error.

I minimized the code the following 3 classes to show the bug:

import org.swiftsuspenders.Injector; import flash.display.Sprite; public class Main extends Sprite { public static var injector:Injector; public function Main() { injector = new Injector(); injector.mapSingleton(ClassA); injector.mapSingleton(ClassB); injector.getInstance(ClassA); } }

public class ClassA {
    [Inject]
    public var classB:ClassB;
    public function ClassA() {
        trace("Constructor", this);
    }
    [PostConstruct]
    public function initialize():void {
        trace("Initialize", this, ", classB instance:", classB);
        Main.injector.getInstance(ClassA); // should not create a new ClassA (will throw an error)
    }
}

public class ClassB {
    public function ClassB() {
        trace("Constructor", this);
    }
    [PostConstruct]
    public function initialize():void {
        trace("Initialize", this);
    }
}

The error looks like:

Constructor [object ClassA] Constructor [object ClassB] Initialize [object ClassB] Initialize [object ClassA] , classB instance: [object ClassB] Constructor [object ClassA] Initialize [object ClassA] , classB instance: [object ClassB] Constructor [object ClassA] Initialize [object ClassA] , classB instance: [object ClassB] Constructor [object ClassA] Initialize [object ClassA] , classB instance: [object ClassB]

Let me know what you think.

Romu

ZackPierce commented 12 years ago

This error (a looping stack overflow) happens because the post-construct method execution happens as part of the same injector.instantiate call that creates the class. In InjectSingletonResult, we don't store the produced instance until the whole injector.instantiate call completes.

The most apparent solution would be to factor out the post-construction portion of the injection process as a new method separate from the main work of injectInto. However, introducing such a possible (though usually optional) separation would make it easier for objects to be accidentally put into states of partially-complete injection/preparation. It might be reasonable to simply declare that an object cannot be considered ready for injection or injector-retrieval until all of its PostConstruct methods have finished.

ZackPierce commented 12 years ago

Also, an even-simpler test case for this scenario. Running it produces a stack overflow error (handily caught by FlexUnit).


class SelfClassRetrievingPostConstruct
{
    [Inject]
    public var injector:Injector;

    public var one:Boolean = false;
    public var retrievedInstance:SelfClassRetrievingPostConstruct;

    public function SelfClassRetrievingPostConstruct()
    {
    }

    [PostConstruct]
    public function methodOne():void
    {
        one = true;
        retrievedInstance = injector.getInstance(SelfClassRetrievingPostConstruct);
    }
}

[Test]
public function injectorResolvesSingletonInstanceReferencesDuringPostConstruct():void
{
    injector.mapValue(Injector, injector);
    injector.mapSingleton(SelfClassRetrievingPostConstruct);
    var selfRetrievingPostConstruct:SelfClassRetrievingPostConstruct = injector.getInstance(SelfClassRetrievingPostConstruct);
    Assert.assertTrue('PostConstruct method executed', selfRetrievingPostConstruct.one);
    Assert.assertStrictlyEquals('When mapped as a singleton, only one instance is created per Class', selfRetrievingPostConstruct, selfRetrievingPostConstruct.retrievedInstance);
}
darscan commented 12 years ago

Will this affect RL2 configs that use [PostConstruct] to perform configuration?