google / guice

Guice (pronounced 'juice') is a lightweight dependency injection framework for Java 11 and above, brought to you by Google.
https://github.com/google/guice
Apache License 2.0
12.51k stars 1.67k forks source link

Difference between @Singleton and .asEagerSingleton() #1834

Open manmedia opened 1 month ago

manmedia commented 1 month ago

Hello,

I have a project where I see my service is failing to initialise.

I am using dropwizard guicey - but it uses Guice underneath, and my question is specific for Guice because the dependency injection is the theme here.

I am attaching the project in question. So that the issue can be reproduced and seen.

The class - TheProblemClass extends AbstractScheduledService (Guava) and implements Managed interface in Dropwizard. The idea is that I ought to be able to start/stop the service upon Main server startup and shutdown respectively. Also, I could schedule a periodic call to one of its methods.

Some things I have noticed.

  1. When TheProblemClass is annotated with @Singleton - it is working fine
  2. When TheProblemClass is NOT annotated with @Singleton - it is always hanging.

In my Module - I have bound it as an eager singleton. My understanding was that I only needed to do this in one place. And the rest should be OK. Have I misunderstood how Singleton Annotation and Module Configs are supposed to be working together? Archive.zip

xvik commented 1 month ago

This is, actually, a consequence of the problem you already reported to guicey. I did not see hangs in the second case, but the problem should be caused by two service instances, created in the second case.

In short, @Singleton annotation and .asEagerSingleton() (.in(Singleton.class) ) declarations would do (almost) the same. But, in your case, there are two bindings: Sample interface (linked to TheProblemClass in privite module) and TheProblemClass (direct). In this case, .asEagerSingleton() affects only one binding and, without direct @Singleton annotation on class, direct class binding would be in prototype scope and so there would be two service instances.

Long version:

Private module declaration:

public class MyGuiceModule extends PrivateModule {
    @Override
    protected void configure() {
        bind(Sample.class).to(TheProblemClass.class).asEagerSingleton();
        expose(Sample.class);
    }
}

Guicey detects target class (TheProblemClass) as an extension (Managed interface) due to configured classpath scan, but guicey did not (currently) detect bindings exposed by private modules and so it bounds this class as untargetted binding:

binder().bind(TheProblemClass.class);

In order to register extension, guicey requests its instance:

injector.getInstance(TheProblemClass.class);

This would cause instance creation from the unetrgetted prototype binding. The second instance is created due to injection point:

@Inject
    public SampleResource(Sample sampleProblem) {
        this.sampleProblem = sampleProblem;
    }

To verify this just add simple constructor to the TheProblemClass:

 public TheProblemClass() {
        System.out.println("CTOR " + this.hashCode());
    }

Without @Singleton annotation, you will see two instances created:

CTOR 1835777333
CTOR 501855493

With singleton annotation, there would be only one instance:

CTOR 1967434886
manmedia commented 1 month ago

@xvik Thanks for the detailed explanation. So there are a few issues:

  1. How the scan discovers or ignores protected classes (Guicey specific) - raised issue in Guicey project
  2. How the Singleton scope is evaluated (Guice specific)

What I understand is that having @SIngleton is sufficient for all the places unless there is a PrivateModule setup. i.e. I should use both .asEagerSingleton and @Singleton if there is an exposure?

xvik commented 1 month ago

Normally, guicey should detect already declared binding and did not register another one.

What I understand is that having @SIngleton is sufficient for all the places unless there is a PrivateModule setup. i.e. I should use both .asEagerSingleton and @Singleton if there is an exposure?

The problem is that there is actually two bindings, so .asEagerSingleton() affects only one binding and another one (created by guicey) remains in prototype scope. From guice perspective, there is no specific behaviour with PrivateModules.

In context of guicey, using @Singleton is "safer" because this way you may be sure that there would be no duplicate instances (no matter how many bindings). Moreover, it makes singleton essence more obvious for anyone reading service class (but it's a personal preference).

.asEagerSingleton() differs from singleton annotation: it will force singleton instance creation in any case (even if noone request it). But, as target service is guicey extension, guicey would instantiate it and so .asEagerSingleton() is not required.

manmedia commented 1 month ago

Normally, guicey should detect already declared binding and did not register another one.

I think this is where we had seen non-reproducible behaviour where we managed to register two instances. For now, I suspect the right thing to do is use @Singleton where warranted - along with asEagerSingleton