From http://www.netbeans.org/nonav/issues/showattachment.cgi/77779/deadlock.txt in issue #159714:
at sun.reflect.GeneratedMethodAccessor32.invoke(Unknown Source)
at $Proxy2.resultChanged(Unknown Source)
As pointed out by vstejskal, it is indeed surprising that calling Lookup.Result.allInstances, which sounds like a
"read-only" method, can in fact trigger changes to be delivered to listeners. This is because
MetaInfServicesLookup.beforeLookup checks whether an interface (call it I.class) is being queried for the first time,
and if so, computes the result and notifies it.
Perhaps the notification should simply be deleted. There is no "change" visible to someone watching a Lookup.Result<I>,
because they would already have triggered search of META-INF/services/I before.
The only use case for this notification that I can think of is if I1 extends I2, an impl is registered using
@ServiceProvider(service=I1.class), someone is listening to Lookup.Result<I2>, and someone else asks for
Lookup.Result<I1>.allInstances() - then the listener on I2 gets notified that some instances of I2 appeared out of
nowhere. But expecting MISL to provide subtypes is a mistake to begin with, since in general you will not get instances
registered under a different interface - you can only get lucky. So not getting notifications if some such serendipitous
instances appear does not seem like a significant bug to me.
Created attachment 85791 [details]
Yes, MISLookup delivers change events inside allInstances. There has already been few bugs closed as wontfix regarding
Jesse, you cannot delete the whole test. You can only delete its listening part. At the end there still need to be two
instances visible for anyone who queries the superinterface. Btw. this needs to be guaranteed even if the result is
wrapped by few ProxyLookup which cache their results.
Unless this is addressed I cannot accept your patch.
Created attachment 85871 [details]
Non-listening portion of test restored
There are still two instances for anyone looking for the supertype. Of course this will not working through a caching
proxy lookup, but that is just too bad - you shouldn't be looking for the supertype to begin with - and anyway not
fixable when we use the observer pattern to implement proxying.
Until the results via ProxyLookup are consistent, I am not going to accept any change of this kind.
I agree firing events from getter is wrong. I can live with delivering no events or events in another dispatch thread.
But results have to stay consistent. Btw. why don't you pass RP.getDefault() or no-op Executor to
MetaInfServicesLookup super class constructor? Imho that shall work and even keep consistency of cached ProxyLookup
(shall to be tested).
Y01 Don't use WeakSet - Lookup classes are independent on the rest of openide.util. Verified by
To Y01 - if we wish to keep this separation then we should create org.netbeans.modules.lookup (with module.jar.dir=lib)
and make org.openide.util depend on it.
Attaching patch which delivers events asynch, which is ugly but probably better than the current state. (I did not find
any super constructor taking Executor, so I'm not sure what you mean by that.)
Ideally no events would be delivered at all unless there was actually someone listening on a strict supertype of the
queried interface. Not sure how that would be implemented. Or maybe this already works by the nature of lookup listeners.
A previous iteration of the code + test failed for me unexpectedly when the NotifyListeners _constructor_ delivered
events to listeners (via AL.R.collectFires), but I cannot reproduce with the current version. You are probably the only
one who understands this code so I will not delve into it.
Created attachment 85933 [details]
Revised patch; events delivered asynch, so ProxyLookup becomes consistent eventually
Re. org-openide-util-lookup.jar - yes, I can create it. Not sure if for 6.8, but report an issue if you feel it is
Integrated into 'main-golden', will be available in build *200908080201* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
User: Jaroslav Tulach <email@example.com>
Log: #169844: Accepting Jesse's patch: Listeners shall not be called from getter like methods