Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Difference between @Singleton and .asEagerSingleton() #1834

Open
manmedia opened this issue Sep 30, 2024 · 4 comments
Open

Difference between @Singleton and .asEagerSingleton() #1834

manmedia opened this issue Sep 30, 2024 · 4 comments

Comments

@manmedia
Copy link

manmedia commented Sep 30, 2024

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

@manmedia manmedia changed the title Difference between Singletone and .asEagerSingleton() Difference between @Singleton and .asEagerSingleton() Sep 30, 2024
@xvik
Copy link

xvik commented Oct 1, 2024

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
Copy link
Author

manmedia commented Oct 1, 2024

@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
Copy link

xvik commented Oct 1, 2024

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
Copy link
Author

manmedia commented Oct 1, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants