This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
From http://www.netbeans.org/nonav/issues/showattachment.cgi/77779/deadlock.txt in issue #159714: ... at org.netbeans.modules.editor.settings.storage.StorageImpl$Filters$1.resultChanged(StorageImpl.java:494) at sun.reflect.GeneratedMethodAccessor32.invoke(Unknown Source) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) at java.lang.reflect.Method.invoke(Method.java:597) at org.openide.util.WeakListenerImpl$ProxyListener.invoke(WeakListenerImpl.java:450) at $Proxy2.resultChanged(Unknown Source) at org.openide.util.lookup.AbstractLookup.notifyListeners(AbstractLookup.java:518) at org.openide.util.lookup.MetaInfServicesLookup.beforeLookup(MetaInfServicesLookup.java:123) at org.openide.util.lookup.AbstractLookup$R.beforeLookup(AbstractLookup.java:1095) at org.openide.util.lookup.ProxyLookup$R.myBeforeLookup(ProxyLookup.java:653) at org.openide.util.lookup.ProxyLookup$R.computeResult(ProxyLookup.java:518) at org.openide.util.lookup.ProxyLookup$R.allInstances(ProxyLookup.java:489) at org.netbeans.modules.editor.settings.storage.StorageImpl$Filters.rebuild(StorageImpl.java:505) at org.netbeans.modules.editor.settings.storage.StorageImpl$Filters.access$400(StorageImpl.java:456) at org.netbeans.modules.editor.settings.storage.StorageImpl$Filters$1.resultChanged(StorageImpl.java:491) ... 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] Suggested patch
Yes, MISLookup delivers change events inside allInstances. There has already been few bugs closed as wontfix regarding this behaviour. 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 http://hudson.apidesign.org/job/lookup/
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
Thanks: core-main#00f02ba81b1c 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 important.
Integrated into 'main-golden', will be available in build *200908080201* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress) Changeset: http://hg.netbeans.org/main-golden/rev/00f02ba81b1c User: Jaroslav Tulach <jtulach@netbeans.org> Log: #169844: Accepting Jesse's patch: Listeners shall not be called from getter like methods