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.48k stars 1.67k forks source link

Lifecycle support #62

Closed gissuebot closed 9 years ago

gissuebot commented 10 years ago

From hani@formicary.net on March 14, 2007 09:22:31

Support for JSR-250 (javax.annotation) would also be very useful for all those people using init() methods and the list.

The mapping I think is:

@PostConstruct -> Called right after construction @Resource -> Analogous to @Inject @PreDestroy -> No concept of destruction, is there? @Generated -> Doesn't apply

Original issue: http://code.google.com/p/google-guice/issues/detail?id=62

gissuebot commented 10 years ago

From james.strachan on July 23, 2010 00:59:24

Hooray!

Whatever Service interface you decide to go with, please make sure there's some kind of ServiceStrategy so we can plug in adapters to it - as there's a zillion different mechanisms out there for basically calling a 'start' and a 'stop' on a bean (JSR 250, Spring's IniitializingBean / DisposableBean then most frameworks add their own API with some kind of start/stop method - the JDK has Closable etc).

Being able to stop a scope would rock too.

gissuebot commented 10 years ago

From sberlin on July 23, 2010 05:36:59

Some thoughts, based on previous lifecycle stuff I've written to cooperate with Guice...

  both startAndWait & startAsync (similarly both stopAsync & stopAndWait) is unnecessary.  xAndWait can be done by calling get() on the returned future of the async method.   I've found an 'initialize' state to be useful -- that is, a state between & State.RUNNING.  (likely two states: State.INITIALIZING & State.INITIALIZED), with a method Future<State> initialize().  This lets a lot of services that are interdependent make sure that they each have a proper setup before starting.  (The service manager would call initialize on everything before starting anything.)   While there should be a granularity on services per service (or key), there should also be a way for a single listener to be notified of all service state changes.   There should be an easy way to schedule periodic Runnables/Callables (similarly to ScheduledExecutorService), to ensure that they start when the service starts & stop when the service stops.  Something easier than delegating to a SecheduledExecutorService, capturing the returned ScheduledFuture in start & stopping it in stop.. because that usually either goes wrong (forgetting null checking), or is ignored (no stopping).  * There should be some concept of service order.  Doesn't need to be exact, but I've found buckets (early, middle, late) to be very helpful.

gissuebot commented 10 years ago

From christianedwardgruber on July 23, 2010 11:33:41

There's no need for the manager to call everything before anything is returned - we have a dependency graph, we can just call startup along the dependency chains.

Anyway, Jesse Wilson and I had a similar conversation about five weeks ago (I think) and came to a similar conclusion about an interface.  I was working up a proposal, but hadn't seen this bug.  I DO think it's important that we not go with the "start everything" approach.  Too much up-front hit, I think.  Wiring should happen up-front, but one point of separating out wiring from config from startup is to keep the costs of these pieces of work separate (and maintainable).

The other thing that needs to happen with any lifecycle system is a strong set of recommendations on where to do what kinds of work, so people have a clue.  So much work is done in providers and constructors to-date, that having good instructions could ease migration to using the lifecycle in a maintainable and sane way.

gissuebot commented 10 years ago

From christianedwardgruber on July 23, 2010 11:34:03

And yes, I'm volunteering. :)

gissuebot commented 10 years ago

From sberlin on July 23, 2010 11:48:47

We have a dependency graph for use/construction dependencies, but not for service requirements.  It's fairly often that a service (Foo) needs other services (Bar, Baz) setup (either initialized or up & running) before Foo can start, despite Foo not having a Guice-visible dependency on Bar/Baz.

gissuebot commented 10 years ago

From christianedwardgruber on July 23, 2010 11:55:00

Why would Foo not have guice visible dependency on Bar/Baz, if Guice is actually creating the wiring graph... I don't mean it has to have a direct visible dep on Bar or Baz, but it should have a sort of transitive dependency on those that Guice, having wired up the graph first, knows about.

Picocontainer does it this way and it works nicely.  Components which need lifecycle implement the start() semantics, and the container starts them in dependency order as-needed.  What am I missing about how Guice handles things?  Or is the lifecycle proposal external to the Injector entirely?

gissuebot commented 10 years ago

From dhanji on July 23, 2010 12:38:07

We discussed this at length, and were pretty convinced that best solution is for the user to manage the dependency order with the help of a CompositeService:

Service.start() // called from main()

CompositeService -> calls Service1.start(); Service2.start(); etc.

gissuebot commented 10 years ago

From sberlin on July 23, 2010 13:10:58

@dhanji -- So long as multiple .start() calls are ignored (the service isn't started twice), and the returned Future from latter calls still works for blocking.. that seems like a good plan.  I'm a little scared about having an insanely large CompositeService -- kind of like an additional Module, except for service ordering instead of object wiring.

@cwg -- It's often the case that a service has no need for a real "wiring" dependency on something else, but it still has a service dependency on it.  It would be ashame, I think, to force an object dependency on it.  This has been what's nice about Guice not including lifecycle as part of a base feature -- it separates lifecycle dependencies from object dependencies.

gissuebot commented 10 years ago

From undeconstructed on July 23, 2010 13:20:33

I have an interest in the Service approach being talked about here, but to me that seems to be a separate issue to how this started out.  Recently I was trying to implement a service architecture on Guice, but ran into various issues that made a quick solution impractical. A major one being the difficulty in finding which bindings are really services, when the interfaces they are bound to do not indicate this. That is, finding implementations that are singletons with the required "Service" interface, even when the bean itself might be bound to a provider method, returning only a public interface for the service it offers.

The easiest way around this was (unfortunately) to switch to Spring, which has two separate lifecycle concepts, 1) injection lifecycle (init, destroy, etc,) 2) context lifecycle (start, stop). These two are well defined, but also pretty simplistic, in that you get only one context lifecycle, that will always start every bean with the right interface, as long is it is defined at the top level in the config.

What I think would be more interesting to experiment with for this use case is a general purpose service framework, built on top of Guice. This layer would be able to offer features such as promotion of some interfaces from an injector as services, while hiding all implemention; more complicated state management (initialized, starting, started, going offline...); or even automatic proxying/remoting.  It might be possible to do something like:

configure() {   bind(Someimpl.class)   bind(InternalThing.class).to(another.class)   bindAsService(MyService.class).to(myserviceimpl.class) }

And then have MyService available to inject into other modules, while the InternalThing binding would only be visible to those locally defined bindings.

However, I don't really see this as a responsibility of Guice - Guice just proviides some mechanisms that could really simplify the work.

gissuebot commented 10 years ago

From christianedwardgruber on July 23, 2010 13:24:54

<sigh>

I was afraid this was where Guice was going to go...

I wholeheartedly disagree that composite-service wtih a separate listing of dependency is the way to go.  For me, a collaborator dependency graph IS the order in which B needs A, and usage-order/start-order implies construction order (as in, if B needs A to be started before it needs to be started, it surely needs A to exist before it needs to exist.  Even if it doesnt' know about A, because it's transitive.

So I don't see it as an artificial dependency, but an absolutely natural one, implicit in the creation graph.

But even if it was artificial, with a CompositeService (which isn't really a "feature" of guice, as I can do this with a Multibinder today), you're basically specifying a separate, duplicate wiring which (as I said) overloads (to an extent) the creation graph.  That's maintaining metadata twice, which (again, to me) is really hard to maintain.

I've long considered the lack of lifecycle a bug, not a feature, and a radically bad design choice, and what I've seen in terms of people adding work to constructors/providers as a "workaround" the missing feature (to me) justifies this view.

I'll dig back through the bug and conversations and see if I can't convince myself otherwise, but I"m afraid I wasn't convinced when I first read the arguments about "we're an injector not a container" and I suspect I won't find much new to convince me.

And I'm sorry if I'm a little snippy in this bug thread, but I had built up hopes and I'm actually really disappointed in this decision, because I really really really want to like Guice, and it's clean in so many other ways.

gissuebot commented 10 years ago

From hani@formicary.net on July 23, 2010 14:14:41

My problem with the Service approach is just the same as it is with every other proposed solution. The attitude is 'we'll come up with something complex to address every single case, then it's up to the community to do the simplification that 95% of apps need'.

Sure, the Service interface would do the job. Except I now have to implement an interface, and I suddenly have to know about concurrency and futures and states.

If I'm the kind of person who knows al that, chances are I'm also clever enough to do all that in my @PostConstruct method.

I accept the religious argument against any external dependencies (or rather, am resigned to it). So how about guice specific versions of the standard annotations? Hell, give them the exact same name and just put them in a guice package. That way the totally awesome google code wouldn't have to be polluted with javax filth.

Also sorry for the snippy tone, probably cos I'm angry at myself for forgetting to celebrate the 3 year anniversary of this issue!

gissuebot commented 10 years ago

From dhanji on July 23, 2010 14:53:55

@60 What do you mean by a implies b ordering, in practical terms? Are you saying you want any class in the injector that impl's Startable to have start() called on it? Trying to understand... certainly don't want you to stop liking Guice!

@hani We can support the javax.* annotations as an optional module using Service. The Service scheme is mainly meant to nail down the conceptual programming model. There's no problem with supporting different annotations.

gissuebot commented 10 years ago

From christianedwardgruber on July 23, 2010 15:51:27

Ok, I have a possible proposal alternative, whipped up by some fellow googlers and I this afternoon, incorporating some of this discussion's pros and cons, and some thoughts from my convo with Jesse.  I'm going to try to put it together into something coherent, with at least sample usage code over the next few days.

Side notes: @62 - Yes, before it's supplied as a dependency, if it implements lifecycle (either the service interface (or annotation or however it's indicated)

@61 - 3 years... heh.  Too funny.

gissuebot commented 10 years ago

From limpbizkit on July 24, 2010 00:51:19

@61: the goal of Guava's service interface is to do the concurrency for you. Clients who want to start services just call start/startAndWait(). Those methods are idempotent and allow you to shutdown a service asynchronously. People defining their own service implement AbstractExecutionThreadService, where they implement startup, run loop, and shutdown methods. http://guava-libraries.googlecode.com/svn/trunk/javadoc/com/google/common/util/concurrent/AbstractExecutionThreadService.html

gissuebot commented 10 years ago

From mcculls on July 27, 2010 01:59:15

@61 FWIW you can use @PostConstruct with Guice if you add the https://code.google.com/p/guiceyfruit/ extension. This uses the same SPI that I used to re-implement Plexus as an extension on top of Guice.

On a side-note, to support Plexus lifecycles I took the simple approach and used an InjectionListener to track instances that implemented the various Plexus lifecycle interfaces - for each instance I ran through the appropriate startup sequence. This worked because for all cases (so far) the startup sequence matched the injection sequence. When the container stopped I just did the calls in reverse.

So basic lifecycle support is already possible using the SPI in trunk - I also think it's better for this to be an extension rather than baked into core, as there will always be someone who wants a different model.

gissuebot commented 10 years ago

From james.strachan on July 27, 2010 02:26:04

@mcculls yes - the problem isn't the 'start' part of lifecycle, its easy to do with the existing hooks. The big problem is trying to do a 'stop/close' or @PreDestroy in a clean way without hacks like forking guice (which guiceyfruit had to do) or writing your own custom Scope objects etc. (FWIW your idea only works if the stop hooks map exactly to the start hooks, which isn't the case in JSR 250).

We should be able to create an Injector, look up some objects then close down all the singletons for example without jumping through too many hoops. (Ditto Request / Session scope in guice-servlet).

Guice knows which objects have been created in a particular scope - they are stored in that scope (thats what a scope is afterall), so its kinda trivial to be able to iterate through a scope so you can close the objects - it just requires a fairly trivial change to guice to be able to do such a thing... https://code.google.com/p/google-guice/issues/detail?id=354 Right now there are only a small subset of real 'stop' lifecycles you can do without hacking your own replacement Scopes, forking guice or solving a small subset through hacks (basically creating your own mirror scope keeping track of all the objects you post process in a start interceptor).

gissuebot commented 10 years ago

From cowwoc%bbs.darktech.org@gtempaccount.com on January 25, 2011 21:55:36

How about implementing this feature incrementally? Does anyone disagree with the fact that the built-in scopes (I'm thinking REQUEST/SESSION scopes here) should clean up all Closeables at the end of each request/session? That seems pretty non-controversial to me. The beautiful part is that invoking close() multiple times is guaranteed to have no-effect so this feature won't conflict with @PreDestroy.

gissuebot commented 10 years ago

From james.strachan on January 26, 2011 00:51:12

I don't see why any particular Scope should be special and be able to do proper lifecycle starting & stopping of resources while others are not. Whether a Scope is application/singleton or page/request/session or some other kind of scope (conversation or whatnot) - its a pretty basic requirement to be able to cleanly start and stop resources without developers having to patch/hack/fork a DI container or jump through silly hoops.

Pico & Spring have supported this for like 5 years; they make it easy for a developer. Its pretty sad there's still a discussion on this 4 year old issue which has been fixed in pretty much all DI engines ever - apart from Guice :(. I still can't fathom the push back. Maybe Guice committers just really get off on saying no? :)

gissuebot commented 10 years ago

From kvaster on November 05, 2011 05:48:34

Telling the truth, I don't understand why guice have no good lifecycle support yet... Mycilla guice is good, but it is not so good as it should be. Errors on container start are not properly handled. i.e. some instances are already started when error occurs. in that case all services should be stopped which were already started...

gissuebot commented 10 years ago

From dhanji on November 27, 2011 14:58:58

You can try Sitebricks's AwareModule as a lightweight lifecycle support for Guice. https://github.com/dhanji/sitebricks/blob/master/sitebricks-web/src/main/java/info/sitebricks/persist/PersistAware.java And the module looks like: https://github.com/dhanji/sitebricks/blob/master/sitebricks-web/src/main/java/info/sitebricks/persist/StoreModule.java

gissuebot commented 10 years ago

From kvaster on November 27, 2011 15:02:26

And what about injector creation errors ? Some services may be already started in case of partially created injector and we defenitely need to stop them...

gissuebot commented 10 years ago

From kvaster on December 24, 2011 11:59:17

Just in case somone interested. mycilla-guice works ok for @PostConstruct and @PreDestroy for me, but I had one problem: guice+mycilla does not work ok when guice fails to create injector (in case some of services failed to start). Theese patches add:

Attachment: gist    mycilla-destroy-on-error.patch    guice-singleton-recreation.patch

gissuebot commented 10 years ago

From crazyboblee on March 13, 2012 22:06:27

This issue is still open? :-)

I'd recommend against adding a sophisticated service lifecycle management framework to Guice. This could be a framework in and of itself. Of course, such a framework could integrate with Guice.

5 years ago, I didn't want Guice to depend on @PostConstruct and @PreDestroy because this would have created a dependency from Guice on Java EE. This would have been unacceptable because core Guice needs to run on platforms like Android.

Today, these annotations are part of core Java (since Java 6). Functionality that depends on these annotations could fail fast when the annotations aren't present (Java 5, Android, etc.). For example, Injector.shutDown() would throw a runtime exception.

Simple support for @PostConstruct and @PreDestory might make since.

It wouldn't make sense to support @PostConstruct alone. It exists because @Resource supports only fields and methods. Guice supports constructors and does not need something like @PostConstruct. However, if we support @PreDestroy, we should almost certainly support @PostConstruct, too, to avoid confusion.

Do we need @PreDestroy support? If so, how exactly should it behave? What level of flexibility do we need with regard to concurrency (asynchronous, parallel), exception handling, invocation ordering, etc.? Does it apply to only objects instantiated by Guice or all objects? What's the compatibility story?

To answer these questions, we need concrete, real world use cases (examples) for @PreDestroy. We need lots of examples, and in each case, @PreDestroy should be the best way of addressing that use case.

Finally, when it comes to the implementation, we need to take care not to introduce performance regressions, even if that means making this an "opt-in" feature. Performance is already bad enough on Android.

gissuebot commented 10 years ago

From cowwoc@bbs.darktech.org on March 13, 2012 22:26:58

Hey Bob,

Glad to see that you're still alive :)

I think that this feature needs to be split into two:

  1. A built-in @PreDestroy behavior that invokes close() on all instances of Closeable.
  2. Support for @PostConstruct, @PreDestroy and other annotations.

Everyone seems to be on-board for #1. There seems to be some controversy surrounding #2. Couldn't we resolve them separately?

gissuebot commented 10 years ago

From crazyboblee on March 14, 2012 10:42:44

I wouldn't use Closeable (#1) here. It would be surprising to me if Guice called close() on my objects just because they happen to implement Closeable. Also, "close" doesn't fit all of the use cases.

If anything, we'd support @PreDestroy, but again, what are the use cases? A looked through the first 20 or so pages of results on koders.com and didn't see one interesting usage. I can try to imagine some:

  1. Closing I/O resources   2. Shutting down threads and executors   3. Proactively freeing native memory   4. Removing external references (listeners, etc.)   ...

@PreDestroy doesn't seem like a good solution to any of these. Use try-finally, try-with-resources, references, explicit shut down logic, etc., instead, and your code will be less error prone and more maintainable. No offense intended to Hani and others than were on JSR-250, but my sense is that @PreDestroy is an annotation in search of a problem.

gissuebot commented 10 years ago

From hani@formicary.net on March 14, 2012 10:45:35

The usecase I have is mostly in getting rid of Providers.

Basically, once everything is wired up (config) I want to then 'start stuff up', this includes making external connections based on bindings, and so on. It's a very good match for @PostConstruct.

@PreDestroy I haven't used. I tend to prefer explicit demarcation lines to denote scope ends, rather than have it done for me.

gissuebot commented 10 years ago

From kvaster on March 14, 2012 11:28:43

Basic use case:

* I want my beans to start once all is configured. Some beans needs to creat thread to process data e.t.c. This can be done in constructors...

gissuebot commented 10 years ago

From sberlin on March 14, 2012 11:45:52

I very much believe that lifecycles like "starting" should be handled explicitly.  Classes should implement 'Service' and can use multibinder to bind into a Set<Service>, and then the Services can be started.  This doesn't feel like dependency injection to me.

gissuebot commented 10 years ago

From eric.jain on March 14, 2012 11:53:09

I can live without @PostConstruct, but without @PreDestroy I have to remember which instances to close, which is error prone. If there is a better way, Id love to hear about it...

public class Application {

  private Injector injector;

  public void onStart() {     injector = Guice.createInjector(new AbstractModule() {       @Override       protected void configure() {         bind(Foo.class).in(Singleton.class);         ...       }     });   }

  public void onStop() {     injector.getInstance(Foo.class).close();     ...   } }

gissuebot commented 10 years ago

From kvaster on March 14, 2012 11:56:01

Services should start in the proper order - all dependencies should start before. Same for stop. So it's really close to dependency injection.

gissuebot commented 10 years ago

From kvaster on March 14, 2012 14:28:33

Telling the truth, I am really satisfied with mycilla guice. BUT guice defenitely should include bug fixes from #676. 1) Guice does not throw errors on null values in injected fields, but it should. 2) Singletons are recreated many many times even if error occurs.

gissuebot commented 10 years ago

From hani@formicary.net on March 14, 2012 14:32:08

Yes, I never use guice 'base', always go with mycilla instead.

gissuebot commented 10 years ago

From christianedwardgruber on March 14, 2012 14:45:50

Apparently I failed to post this to the group when I replied to this thread on the dev list. :(  Anyway,

I agree that in all cases I've encountered in my own industry experience, the dependency-construction wiring order was also the correct order for service start-up, and there are efficiencies that can be pulled out in terms of starting up services in parallel where they're independent, etc.  However, in many cases it wasn't "necessary" it was "one correct ordering" of service startup.

Besides,  in this case, what Guice needs to provide is additional API to walk the graph of objects along the dependency wiring, so an extension can do the work.  Service support doesn't need to be baked in to the core, and probably shouldn't, so those who aren't using such don't pay the performance penalty.

Put simply - Guice manages the metadata of the dependency graph related to construction/injection order.  And it also provides the activities of actually constructing the objects and injecting them.  And it provides the intermediate steps of registering bindings, building a wiring order, and keeping that dependency graph straight.  I think we can solve this with extensions, for those around whom the startup order is important.

I'm going to repeat Bob's call for use-cases.  Can we have a comprehensive set of use-cases for service startup that needs more than Sam B.'s Set<Service> or Map<StartOrdinal, Service> (inet.d style startup)?  My historical examples are all from code I can't admit to existing, and in the end we opted for alternatives that did have service startup built-in.  But if we're going to put engineering resources into it, either showing how you would have written a piece of real code that actually needs wiring order, and how you worked around, and why that workaround sucks - that would be helpful.  Also helpful would be for forks/extensions to Guice that have service facilities, to help us understand what they needed from within Guice to implement, and we can talk about how to create an API to support that.

gissuebot commented 10 years ago

From francisdb on March 14, 2012 14:49:54

Ain't this more about proper shutdown than startup?

gissuebot commented 10 years ago

From kvaster on March 14, 2012 15:02:55

Startup can be handled via constructors. This is really more about shutdown.

gissuebot commented 10 years ago

From christianedwardgruber on March 14, 2012 15:16:57

Ok - sorry - shutdown was the key part of this, but startup handled by constructors?  That's really not ideal.  I don't think I'm comfortable with THAT being the recommendation on how to do initialization of services with Guice.

gissuebot commented 10 years ago

From mcculls on March 14, 2012 15:35:33

FYI, it's been really easy to make a lifecycle extension now the ProvisionListener API has been added: https://code.google.com/p/google-guice/source/browse/core/src/com/google/inject/spi/ProvisionListener.java We use this to add Plexus lifecycle support (the classic Initializable/Startable/Disposable interfaces) https://github.com/sonatype/sisu/blob/master/sisu-inject/containers/guice-plexus/guice-plexus-lifecycles/src/main/java/org/sonatype/guice/plexus/lifecycles/PlexusLifecycleManager.java#L129 The great thing about ProvisionListener is that you can use it to track dependency chains, which helps you decide the starting order of components. Note: with Plexus you can't simply just start a component on the constructor call because of cycles (which exist in legacy code) and because components use a lot of field injection.

gissuebot commented 10 years ago

From kvaster on March 14, 2012 15:39:34

Please, take a look on: https://code.google.com/p/google-guice/issues/detail?id=676 This bug is stop point when working with ProvisionListener for startup/shutdown.

gissuebot commented 10 years ago

From sberlin on March 14, 2012 15:40:33

(FYI, ProvisionListener is new in Guice head & not in a released version.  But +1 to using it.  It's extremely useful for stuff like this. We also use it to trace loading times of objects, etc..)

gissuebot commented 10 years ago

From mcculls on March 14, 2012 15:50:06

Re #676 our lifecycle support fails-fast so that hasn't been an issue for us, but I can see how it could affect others - can you perhaps add a testcase to demonstrate each scenario and verify the proposed fix?

gissuebot commented 10 years ago

From cowwoc@bbs.darktech.org on March 14, 2012 16:05:39

Bob,

I'm not suggesting that Closeable replace @PreDestroy. I'm saying: don't make us implement @PreDestroy for Closeable. Guice should provide this reasonable default behavior.

What's the harm in auto-closing Closeable instances once their enclosing scope is destroyed? There is no harm in doing so because the specification explicitly states "If the stream is already closed then invoking this method has no effect."

Common use-case: Closing database connections (if necessary) to prevent leaks.

gissuebot commented 10 years ago

From crazyboblee on March 14, 2012 16:11:28

> What's the harm in auto-closing Closeable instances once their enclosing scope is destroyed?

Maybe I don't want the object to be closed.

gissuebot commented 10 years ago

From cowwoc@bbs.darktech.org on March 14, 2012 16:33:34

Bob,

Hehe. Okay then :) Please provide a reasonable use-case where an out-of-scope Closeable object shouldn't be closed.

gissuebot commented 10 years ago

From crazyboblee on March 14, 2012 16:42:38

Sorry, cowwoc. Guice is explicit and not surprising. Someone maintaining your code and trying to figure out if/how a resource gets closed would no doubt be surprised to find out that Guice automagically does it for them.

gissuebot commented 10 years ago

From crazyboblee on March 14, 2012 17:11:10

I think closing/destroying objects when they go out of scope is a red herring.

It sounds like people really want the ability to start and stop per injector services in the right order. While the lifecycle of the service is the same as the injector's, the object isn't necessarily in singleton scope–maybe the service isn't eligible for injection into other objects (see Binder.requestInjection()).

@PostConstruct and @PreDestroy seem like a bad fit here. First, they don't speak to starting and stopping services. Second, and less important than the naming and semantics, using annotations will be slower and more error prone than using a marker interface.

How do we decide which objects are eligible to be started? Strawman: any object that's created or injected during injector creation. This includes eager singletons, objects passed to requestInjection(), and any transitive dependencies. Note: This could get a little weird if you use DEVELOPMENT vs. PRODUCTION stages.

One could use an InjectionListener named ServiceManager to collect any object that implements Guava's Service interface. After injector creation, you call ServiceManager.start(). ServiceManager stops collecting objects (by ignoring any further callbacks), and then it iterates over the objects in the order in which they were created–so you can use dependencies to control the order services are started in–and it starts each service.

When you're ready to stop, call ServiceManager.stop(). It'll stop the services in the reverse order.

Once Guice supports concurrent singleton injection (by inspecting the dependency graphs), ServiceManager could keep track of which threads the callbacks come in on. Then it could make sure that objects instantiated in the same thread are also started in the same thread.

gissuebot commented 10 years ago

From hani@formicary.net on March 14, 2012 17:38:09

So checking for a @PostConstruct is a performance hit when you're already going through all methods and fields checking for @Inject? Why is it any slower?

The flow as I see it is, if guice creates an object, it is responsible, once the full object graphs has been constructed, for calling @PostConstruct. I don't see it getting weird with things like .injectMembers() and .requestInjection(), in those cases @PostConstruct would not apply because guice did not construct the object. Admittedly with stages it might get weird, but nothing that well specified behaviour cannot address. For a novice/beginner dev, there are plenty of non-intuitive and cryptic behaviours and features in Guice, but they're pretty well specified so it's not terrible.

Keep in mind also that all this is possible. I have all the extra behaviour I'd like in Mycilla Guice. There's just some fundamental issue I must be misunderstanding as to why it's so unthinkable for Guice to get those features.

The 'prove we need it argument' is not very compelling anymore, 100 comments and 5 years on from the initial request. I'd like to reverse the challenge, and ask for a non-core Guice developer to argue we absolutely should not have it!

Clearly all arguments in favor of this feature are not ones that have resonated with any Guice developers, so just close it and say it'll never happen.

gissuebot commented 10 years ago

From crazyboblee on March 14, 2012 17:56:52

Iterating over methods and checking for an annotation is an order of complexity slower than "instanceof Service," especially if you didn't already need to reflect on those methods. This is just to say that it would be nice to not force this overhead on Android apps.

Why would we only start instances that Guice instantiated? Seems like an arbitrary limitation.

To be fair, I think you're asking for something different from what everyone else is asking for. Other people want to start/stop services, orthogonally to injector creation. That makes some sense to me, but needs further specification. For example, which objects exactly are eligible to be started?

You've said that you don't care about stopping things and you just want @PostConstruct. This does not make sense to me because @PostConstruct is barely different from a constructor. Also, if you just want @PostConstruct support, it's trivial to hook in with an InjectionListener already.

gissuebot commented 10 years ago

From endre.stolsvik on March 14, 2012 19:46:47

Quick question: Have Guice gotten intelligent about the order in which it instantiates objects?

Earlier, it just did one or the other random order, not caring about each instance dependencies, creating those annoying proxies when it got to any dependency it didn't already have available.

Then one got the option of specifying that proxies should not be used.

However, the order hadn't gotten any smarter, so one still could arbitrarily crash into the ordering-problem, only now it stops with exception, not creating the proxy.

This even though a simple reordering of the flow would have fixed it.

First create the leafs; the ones without dependencies. Then work your way up: Create instances whose dependencies at this point be fulfilled. If you at some point iterate through the entire list of "to be instantiated" w/o instantiating any, then you have a dependency-loop.

Loops are only evaluated by constructors. Fields and methods can always be injected afterwards.

.. which leads to my point #1 about stuff like @PostConstruct..

gissuebot commented 10 years ago

From endre.stolsvik on March 14, 2012 19:52:41

You need @PostConstruct because that would only be invoked after ALL dependencies have been injected - both the constructor and field/methods.

This because you might need some service of some injected instance to start up. And that instance was injected by means of field injection. No good with constructor startup then.

Also, according to JCIP IIRC, one shall not leak unfinished objects. In a constructor, you could e.g. register yourself with the OnIncomingUserEventService, and then your object instantiation crashes with a NPE for some other reason. Therefore, thou shall not ever do "startup stuff" in a constructor. Only construct yourself, do not e.g. attach yourself to other object graphs.

gissuebot commented 10 years ago

From endre.stolsvik on March 14, 2012 20:12:46

When you folks create features out of this request, please enable me to do custom initialization.

I have always ended up in needing several initialization phases, not only a single @PostConstruct.

What I've needed, is at least a "now you've got all your dependencies and configuration, do your init code", and then a "now you can do listener registration with each other".

So my ideal solution to this is to get a list (preferably in instantiation order) so that I can invoke my different init methods on the relevant objects. This could be fetched by user code when the injector was finished setting up the entire graph (and have invoked @PostConstruct?) - and then the user could would do what life cycling it needs to do.

What I currently do, is to have a LifeCycle service which instances can depend on. There is also the LifeCycled interface. The objects register with the LifeCycle service, either in constructor (which is bad according to JCIP), or in the method injecting the LifeCycle service (better). After injector is finished, I run through all the instances that have registered with the LifeCycle service, all of which shall implement the LifeCycled interface. I then invoke method "phase1" on all objects, then method "phase2" on all objects, etc. (On application shutdown, I have corresponding phase-destroy methods on the LifeCycled interface, which are invoked in the opposite order).