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.
Summary: | MetaInfServicesLookup delivers change events inside Lookup.Result.allInstances | ||
---|---|---|---|
Product: | platform | Reporter: | Jesse Glick <jglick> |
Component: | Lookup | Assignee: | Jaroslav Tulach <jtulach> |
Status: | RESOLVED FIXED | ||
Severity: | blocker | CC: | vstejskal |
Priority: | P3 | Keywords: | THREAD |
Version: | 6.x | ||
Hardware: | All | ||
OS: | All | ||
Issue Type: | DEFECT | Exception Reporter: | |
Bug Depends on: | |||
Bug Blocks: | 159714 | ||
Attachments: |
Suggested patch
Non-listening portion of test restored Revised patch; events delivered asynch, so ProxyLookup becomes consistent eventually |
Description
Jesse Glick
2009-08-05 00:01:13 UTC
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 |