Unintended multiple entry point calls on MonoBehaviour

[RamiAhmed]

Hi,

Thanks for building and maintaining VContainer - so far we're really happy to use it, finding it a good improvement over Zenject.

However, we have run into a problem that we suspect is a bug in the framework. It seems that MonoBehaviours that implement "entry point interfaces" (e.g. IInitializable, IStartable, IPostStartable, etc.) get called multiple times - once per child scope loaded. [Inject] decorated methods also appear to suffer from the issue of being called once per child scope loaded.

So binding a MonoBehaviour that implements an entry point interface will have its method called multiple times, despite its scope never being unloaded or reloaded. You don't even need separate scenes to replicate it, as its only tied to loading more VContainer child scopes it seems. Thus MonoBehaviours bound in a "root" / "grand parent" scope will have their entry points called 3 times (one for itself, one for the parent and one for the child).

At a glance it seems that this issue is a duplicate of this issue, but since that has been closed and we are still replicating the bug in v1.6.3 it seems that either the issue was not fully resolved, or that it has reappeared. Also that issue appears to have been caused by bindings in the child scope, whereas here the issue seems to stem from bindings in 'parent' scopes.

Here's a minimal test project where I've set up a case to replicate the behaviour: VContainerTestProject.zip

Steps to reproduce (already set up in the above project which can be downloaded and played out of the box):

1. Install latest version of VContainer from UPM in a new, empty Unity project

2. Set up three game objects which will have each their own LifetimeScope implementation in a scene, and a fourth game object for the behaviours to bind.

3. Implement abstract base behaviour and child implementations, one for each scope, then attach all three behaviours to the BehaviourTest game object:

using System;
using UnityEngine;
using VContainer;
using VContainer.Unity;

public abstract class BaseBehaviourTest : MonoBehaviour, 
    IInitializable, 
    IPostInitializable, 
    IStartable, 
    IPostStartable,
    IDisposable
{
    private int _injectedCounter;
    private int _initializeCounter;
    private int _postInitializeCounter;
    private int _startCounter;
    private int _postStartCounter;
    private int _disposeCounter;

    [Inject]
    public void Inject()
    {
        Debug.Log($"{this}(id: {GetInstanceID()}) {nameof(Inject)} called {++_injectedCounter} times");
    }

    void IInitializable.Initialize()
    {
        Debug.Log(
            $"{this}(id: {GetInstanceID()}) {nameof(IInitializable.Initialize)} called {++_initializeCounter} times");
    }

    void IPostInitializable.PostInitialize()
    {
        Debug.Log(
            $"{this}(id: {GetInstanceID()}) {nameof(IPostInitializable.PostInitialize)} called {++_postInitializeCounter} times");
    }

    void IStartable.Start()
    {
        Debug.Log($"{this}(id: {GetInstanceID()}) {nameof(IStartable.Start)} called {++_startCounter} times");
    }

    void IPostStartable.PostStart()
    {
        Debug.Log(
            $"{this}(id: {GetInstanceID()}) {nameof(IPostStartable.PostStart)} called {++_postStartCounter} times");
    }

    void IDisposable.Dispose()
    {
        Debug.Log($"{this}(id: {GetInstanceID()}) {nameof(IDisposable.Dispose)} called {++_disposeCounter} times");
    }
}

public class RootBehaviourTest : BaseBehaviourTest
{
}

public class ParentBehaviourTest : BaseBehaviourTest
{
}

public class ChildBehaviourTest : BaseBehaviourTest
{
}

4. Then implement the lifetime scopes, each binding their behaviour implementation:

using VContainer;
using VContainer.Unity;

public class RootLifetimeScope : LifetimeScope
{
    protected override void Configure(IContainerBuilder builder)
    {
        builder.RegisterComponentInHierarchy<RootBehaviourTest>().AsImplementedInterfaces();
    }
}

public class ParentLifetimeScope : LifetimeScope
{
    protected override void Configure(IContainerBuilder builder)
    {
        builder.RegisterComponentInHierarchy<ParentBehaviourTest>().AsImplementedInterfaces();
    }
}

public class ChildLifetimeScope : LifetimeScope
{
    protected override void Configure(IContainerBuilder builder)
    {
        builder.RegisterComponentInHierarchy<ChildBehaviourTest>().AsImplementedInterfaces();
    }
}

5. Set up root -> parent -> child lifetime scope relationship setting

6. Press play and you should see how many times each behaviour has its entry points and inject methods called in the console log, e.g.:

So, as can be seen from the above - the behaviour bound in the root / grand parent scope has its entry points and inject methods called thrice. The behaviour bound in the parent scope has its entry points and inject methods called twice, and the child behaviour methods in the child scope are invoked (correctly) once.

This bug is a serious pain for us unfortunately and means that we've had to make workarounds. POCO/Non-MonoBehaviour implementations do not appear to suffer from this issue.

Let me know if there's any more information I can provide to help. Thanks in advance!

[stenyin]

I think I may have found the issue. It seems to be related to RegisterComponentInHierarchy using Lifetime.Scoped by default. This could be causing the multiple calls each time a new child scope is loaded. I’m not entirely certain if this is the root cause, but it appears likely.

Would using Lifetime.Singleton instead resolve the issue, or is there another recommended approach to prevent these repeated injections in child scopes?

[Geminior]

Thanks for looking into this. Your conclusion makes sense, however unfortunately there is no option to change the lifetime to singleton when calling RegisterComponentInHierarchy.

[stenyin]

I believe that entry point interfaces might not be ideal for use with MonoBehaviour components, as the pure C# entry points are actually intended as a replacement for MonoBehaviour. Using entry points on MonoBehaviour can lead to issues with lifecycle management, especially when dealing with scope hierarchies, as seen here.

[Geminior]

1. As for entry points, we have been using IPostStartable to be sure injection has taken place, however, it seems simply moving that logic to Start() works just fine.

One discovery from that refactor, is that calling AsImplementedInterfaces() on a registration of a monobehaviour with zero interface implementations, results in an index out of range exception, e.g.

builder.RegisterComponentInHierarchy<ComponentWithNoInterfaces>().AsImplementedInterfaces()

this is due to registrationBuilder.InterfaceTypes being empty but not null in the following snippet from RegisterComponentInHierarchy

container.Resolve(
                        registrationBuilder.InterfaceTypes != null
                            ? registrationBuilder.InterfaceTypes[0]
                            : registrationBuilder.ImplementationType
                    );

2. The issue also applies to injection, not just entry points. And we can't get around that issue. We have monobehaviours that are intended to be singletons in a parent scope, but they are reinjected in child scopes, due to, as you found, the default (and only possible) registration being scoped. So it makes sense to allow monobehaviours to be registered as singletons.

[RamiAhmed]

It would be cool if RegisterComponentInHieararchy allowed us to specify the lifetime, I think.

[xandudex]

I had same issue, and researched a lot of code, finally get to multi-injection into same object, that was so strange. The problem was really in RegisterComponentInHieararchy, Changed to RegisterComponent and provided instance with SerializedField solved the issue. One more thing: in Editor was no issues, but was in Android build

[hadashiA]

I have read through the discussion of this issue.

First, for RegisterComponentInHierarchy, I would change Lifetime to Singleton. I think you are right that if I don't do so, there is a risk that the child scope will execute Inject in duplicate.

(This would be a destructive change and would probably result in an error if the same type was registered multiple times, but I think there is a workaround as follows.

builder. Register<IReadOnlyList<YourBehaviour>> (c => 
{ 
 return UnityEngine.Object. FindObjectsOfType<YourBehaviour> (); 
}, Lifetime. Singleton);.

On the other hand, there is probably no need to use the EntryPoint interface for MonoBehaviour, and that was not the intended use.

[RamiAhmed]

The main reason for using the entry points is because we have more reliable sequencing - e.g. we can use the "Post" versions to do work that depends on other work being initialized first, so basically more fine-grained and reliable sequence compared to Unity's methods.

I think the ideal would be to allow specifying lifetime on calls to RegisterComponentInHierarchy, could be an optional parameter defaulting to Singleton (or Scoped if you don't want to introduce a potentially breaking change).

Thanks for getting back to us on this!