Unfortunately I don't think that optional would help. Optional means
that if there is NO matching descriptor for the injection point that we
can leave it null. In this case there IS a matching descriptor for the
injection point but it creates a cycle. I still think the best way to
solve the cycle is by using an javax.inject.Provider.
By the way, I have written a test that does show the hang with cycles
and Immediate services, and have fixed it (in that it'll now throw an
exception). That probably doesn't help you in this case though...
I do think your use case of porting over the Guice injection annotations
is interesting... we might want to think about handling things like
optional=true inside other injection annotations...
On 11/19/2014 9:59 AM, Mirko Raner wrote:
> Let me provide some more details...
> In the original code, the registerHandler(...) method is annotated
> with @com.google.inject.Inject(optional=true), in other words,
> registering a handler is not considered part of the essential object
> initialization, so it makes sense that Guice readily constructs the
> object graph.
> As you can imagine, this is also one of the problems that we're
> struggling with on our path from Guice to HK2. To complicate matters,
> the code to be injected is using Guice annotations instead of the
> standard JSR-330 ones, and it is code-generated and somewhat out of
> our control. To solve this, we have also created a custom injection
> resolver that deals with com.google.inject annotations. Unfortunately,
> HK2 does not allow us to completely emulate the semantics of Guice's
> @Inject(optional=true).
> Would you say that the example would be valid if it had an @Optional
> annotation on the registerHandler(...) method?
>
>
> On Wed, Nov 19, 2014 at 7:43 AM, john wells <john.wells_at_oracle.com
> <mailto:john.wells_at_oracle.com>> wrote:
>
> But what that sequence would mean is that RepositoryClient is
> getting injected with a not-fully-initialized ServiceClient. The
> indeterminate state of ServiceClient could be a problem, since it
> is not ready to be used (obviously, its @PostConstruct has also
> not been called, since that must be called after the initializer
> methods).
>
> Both CDI and hk2 define this to be an error, so that they can say
> that they only inject fully initialized services.
>
> As I said before though, this should not cause hk2 to hang. I'll
> look into that today.
>
>
> On 11/18/2014 10:25 AM, Mirko Raner wrote:
>> Thanks for the prompt response, John!
>> Actually, this particular object graph can be instantiated
>> without proxies or providers:
>>
>> * ServiceClient s = new ServiceClientImpl();
>> * RepositoryClient r = new RepositoryClientImpl(s);
>> * MessageHandler m = new MessageHandlerImpl(r);
>> * s.registerHandler(m);
>>
>> It turns out that that's exactly what Guice ends up doing in this
>> situation.
>> We do not have an ImmediateErrorHandler defined, but running the
>> application in the debugger revealed that ImmediateHelper is
>> blocked on a wait() in line 147, and the corresponding notify()
>> is on the same thread.
>> BTW, we're using version 2.3.0-b09.
>>
>> Mirko
>>
>>
>> On Mon, Nov 17, 2014 at 7:22 PM, john wells
>> <john.wells_at_oracle.com <mailto:john.wells_at_oracle.com>> wrote:
>>
>> So, I don't even know how Guice could possibly break this
>> cycle. There is no instance that can be created such that
>> the incoming parameter can be initialized properly. That
>> being said, hk2 should certainly NOT hang here, but instead
>> should detect this very simple cycle. I'll look into that,
>> because it should truly throw an exception in this case,
>> almost immediately. (Actually, I wonder if it IS throwing an
>> exception... do you have a ImmediateErrorHandler
>> <https://hk2.java.net/2.4.0-b06/apidocs/org/glassfish/hk2/utilities/ImmediateErrorHandler.html>
>> defined?)
>>
>> The proper way to fix this (and this is why JSR-330 invented
>> Provider in the first place) is to use a Provider (and also,
>> do NOT use provider.get() in your constructor, since that
>> would also have the same cycle).
>>
>> Only one of your three implementation classes need to use it,
>> so picking one at random to break the cycle, it'd be like this:
>>
>> *class* ServiceClientImpl *implements* ServiceClient {
>>
>> *private*Provider<MessageHandler> _handler_;
>>
>> @Inject
>>
>> *void* registerHandler(Provider<MessageHandler> _handler_) {
>>
>> *this*.handler = handler;
>>
>> }
>>
>> }
>>
>> Obviously, other fixes would include using proxies, but I
>> agree that can get messy and bring in stuff you may not want
>> to have there.
>>
>>
>> On 11/17/2014 6:15 PM, Mirko Raner wrote:
>>>
>>> Hi all,
>>>
>>>
>>> I'm currently dealing with migrating some code from Guice to
>>> HK2, and we are running into some troubles with circular
>>> dependencies in @Immediate scope.
>>>
>>> Please consider the following interfaces and implementations:
>>>
>>>
>>> *interface* MessageHandler _{_
>>>
>>> _}_
>>>
>>> *interface* RepositoryClient _{_
>>>
>>> _}_
>>>
>>> *interface* ServiceClient _{_
>>>
>>> _}_
>>>
>>> *class* MessageHandlerImpl *implements* MessageHandler {
>>>
>>> *private* RepositoryClient _handler_;
>>>
>>> @Inject
>>>
>>> MessageHandlerImpl(RepositoryClient handler) {
>>>
>>> *this*.handler = handler;
>>>
>>> }
>>>
>>> }
>>>
>>> *class* RepositoryClientImpl *implements* RepositoryClient {
>>>
>>> *private* ServiceClient _client_;
>>>
>>> @Inject
>>>
>>> *public* RepositoryClientImpl(ServiceClient client) {
>>>
>>> *this*.client = client;
>>>
>>> }
>>>
>>> }
>>>
>>> *class* ServiceClientImpl *implements* ServiceClient {
>>>
>>> *private* MessageHandler _handler_;
>>>
>>> @Inject
>>>
>>> *void* registerHandler(MessageHandler _handler_) {
>>>
>>> *this*.handler = handler;
>>>
>>> }
>>>
>>> }
>>>
>>>
>>> In particular, please note that there is effectively a
>>> circular dependency between the three implementation
>>> glasses. Not good, definitely a code smell, but also not the
>>> end of the world, and, for purposes of this discussion,
>>> let's just say it cannot easily be avoided.
>>>
>>> In the existing Guice code, there are no problems with
>>> instantiating the classes. Guice figures out the correct
>>> order to instantiate the classes and to break the cycle; no
>>> proxies or other trickery are involved under the hood, AFAICT.
>>>
>>>
>>> To translate the Guice code to HK2, we replaced
>>> .asEagerSingletion() with .in(Immediate.class). It is my
>>> understanding that this is HK2's closest equivalent of eager
>>> singletons.
>>>
>>> Now, if we try to instantiate the objects with HK2, we run
>>> into a deadlock inside the ImmediateHelper class. The
>>> following JUnit test demonstrates the issue; Guice produces
>>> the correct object graph, whereas HK2 times out:
>>>
>>>
>>> *import* org.glassfish.hk2.api.Immediate;
>>>
>>> *import* org.glassfish.hk2.api.ServiceLocator;
>>>
>>> *import* org.glassfish.hk2.utilities.ServiceLocatorUtilities;
>>>
>>> *import* org.glassfish.hk2.utilities.binding.AbstractBinder;
>>>
>>> *import* org.junit.Test;
>>>
>>> *import* com.google.inject.AbstractModule;
>>>
>>> *import* com.google.inject.Guice;
>>>
>>> *import* com.google.inject.Injector;
>>>
>>> *import**static* org.junit.Assert./assertNotNull/;
>>>
>>> *public**class* CircularDependencyInjectionHK2vsGuice {
>>>
>>> @Test(timeout=5000)
>>>
>>> *public**void* testHK2Immediate() {
>>>
>>> ServiceLocator serviceLocator =
>>> ServiceLocatorUtilities./createAndPopulateServiceLocator/();
>>>
>>> ServiceLocatorUtilities./enableImmediateScope/(serviceLocator);
>>>
>>> AbstractBinder binder = *new* AbstractBinder() {
>>>
>>> @Override
>>>
>>> *protected**void* configure() {
>>>
>>> bind(ServiceClientImpl.*class*).to(ServiceClient.*class*).in(Immediate.*class*);
>>>
>>> bind(MessageHandlerImpl.*class*).to(MessageHandler.*class*).in(Immediate.*class*);
>>>
>>> bind(RepositoryClientImpl.*class*).to(RepositoryClient.*class*);
>>>
>>> }
>>>
>>> };
>>>
>>> ServiceLocatorUtilities./bind/(serviceLocator, binder);
>>>
>>> MessageHandler messageHandler =
>>> serviceLocator.getService(MessageHandler.*class*);
>>>
>>> ServiceClient serviceClient =
>>> serviceLocator.getService(ServiceClient.*class*);
>>>
>>> /assertNotNull/(messageHandler);
>>>
>>> /assertNotNull/(serviceClient);
>>>
>>> }
>>>
>>> @Test(timeout=5000)
>>>
>>> *public**void* testGuiceEagerSingleton() {
>>>
>>> AbstractModule module = *new* AbstractModule() {
>>>
>>> @Override
>>>
>>> *protected**void* configure() {
>>>
>>> bind(ServiceClient.*class*).to(ServiceClientImpl.*class*).asEagerSingleton();
>>>
>>> bind(MessageHandler.*class*).to(MessageHandlerImpl.*class*).asEagerSingleton();
>>>
>>> bind(RepositoryClient.*class*).to(RepositoryClientImpl.*class*);
>>>
>>> }
>>>
>>> };
>>>
>>> Injector injector = Guice./createInjector/(module);
>>>
>>> MessageHandler messageHandler =
>>> injector.getInstance(MessageHandler.*class*);
>>>
>>> ServiceClient serviceClient =
>>> injector.getInstance(ServiceClient.*class*);
>>>
>>> /assertNotNull/(messageHandler);
>>>
>>> /assertNotNull/(serviceClient);
>>>
>>> }
>>>
>>> }
>>>
>>> Is this a bug in HK2's Immediate resolution code? Should I
>>> file a JIRA issue?
>>>
>>> Or am I doing something else wrong that sends HK2 into a
>>> deadlock?
>>>
>>>
>>> Thanks,
>>>
>>>
>>> Mirko
>>>
>>>
>>>
>>> This email and any attachments may contain information which
>>> is confidential and/or privileged. The information is
>>> intended exclusively for the addressee and the views
>>> expressed may not be official policy, but the personal views
>>> of the originator. If you are not the intended recipient, be
>>> aware that any disclosure, copying, distribution or use of
>>> the contents is prohibited. If you have received this email
>>> and any file transmitted with it in error, please notify the
>>> sender by telephone or return email immediately and delete
>>> the material from your computer. Internet communications are
>>> not secure and Lab49 is not responsible for their abuse by
>>> third parties, nor for any alteration or corruption in
>>> transmission, nor for any damage or loss caused by any virus
>>> or other defect. Lab49 accepts no liability or
>>> responsibility arising out of or in any way connected to
>>> this email.
>>>
>>
>>
>>
>> This email and any attachments may contain information which is
>> confidential and/or privileged. The information is intended
>> exclusively for the addressee and the views expressed may not be
>> official policy, but the personal views of the originator. If you
>> are not the intended recipient, be aware that any disclosure,
>> copying, distribution or use of the contents is prohibited. If
>> you have received this email and any file transmitted with it in
>> error, please notify the sender by telephone or return email
>> immediately and delete the material from your computer. Internet
>> communications are not secure and Lab49 is not responsible for
>> their abuse by third parties, nor for any alteration or
>> corruption in transmission, nor for any damage or loss caused by
>> any virus or other defect. Lab49 accepts no liability or
>> responsibility arising out of or in any way connected to this email.
>>
>
>
>
> This email and any attachments may contain information which is
> confidential and/or privileged. The information is intended
> exclusively for the addressee and the views expressed may not be
> official policy, but the personal views of the originator. If you are
> not the intended recipient, be aware that any disclosure, copying,
> distribution or use of the contents is prohibited. If you have
> received this email and any file transmitted with it in error, please
> notify the sender by telephone or return email immediately and delete
> the material from your computer. Internet communications are not
> secure and Lab49 is not responsible for their abuse by third parties,
> nor for any alteration or corruption in transmission, nor for any
> damage or loss caused by any virus or other defect. Lab49 accepts no
> liability or responsibility arising out of or in any way connected to
> this email.
>