Community
Participate
Working Groups
I think some of us has already stumbled about the dread "IllegalStateException: The instance data location has not been specified yet". One culprit is DataArea.getStateLocation that goes mad if it is called "to early" (even though there is literally no way for a plugin to *know* when it is to early. My suggestion is to providing variants of this methods that instead of returning the DataArea itself return a CompletableFuture<DataArea> that has the following properties: If everything is setup returns a (completed) CompletableFuture that contains the DataArea. If not returns an (incomplete) CompletableFuture that is held in an internal list. As soon as the initializeLocation is done successfully, completes all pending futures with the now initialized and ready to use DataArea. Of course this will then require to make such things available downstream in a similar manner e.g. ResourcesPlugin.getWorkSpace and ResourcesPlugin.getPlugin need similar methods that allow to deferred any action to a later point in time.
(In reply to Christoph Laeubrich from comment #0) > One culprit is DataArea.getStateLocation that goes mad if it is called "to > early" (even though there is literally no way for a plugin to *know* when it > is to early. > > My suggestion is to providing variants of this methods that instead of > returning the DataArea itself return a CompletableFuture<DataArea> Not sure if OSGI services could offer an elegant way to consume that data for clients (without futures etc)? > Of course this will then require to make such things available downstream in > a similar manner e.g. ResourcesPlugin.getWorkSpace and > ResourcesPlugin.getPlugin need similar methods that allow to deferred any > action to a later point in time. Not sure what do you propose - ResourcesPlugin.getWorkSpace() returning a Future? Here too, may be OSGI patterns could be used if needed? Additionally, in most code I've seen the workspace was used immediately, so no point to provide a deferred API for few "special" clients.
I think a better way would be to move the services to DS instead, but there's challenges with that at the moment in some circular dependencies. What you'd want to do is (for compatibility) call getStateLocation() and have that look up a state area through DS, possibly blocking and then returning it when it exists (or after a timeout, returning the same error). The problem is there's still way too many activators which are triggered when classes are loaded, which in turn tries to activate the thing that's now blocking on the data area, which then leads to deadlock. I've tried to explore this in a few places but the challenges are non-trivial. Even if you have a new API that returns a Future (which is good) you'd still need to have the backwards compatible solution, so if you have: Future<DataArea> getStateLocationFuture() { ... } DataArea getStateLocation() { return getStateLocationFuture().get() } then it's still going to block for those backward futures. I've been working through removing Activators (which will remove some subset of the issues) but in reality the only way we can fix the problem is to replace it with a DS lookup, and that requires that clients use it are DS, which requires clients of those are DS ... It's not insurmountable but it is doable.
@Andrey the main problem is that referencing ANY class from the "ResourcesPlugin" packages currently can cause the issue even though ResourcesPlugin itself is never called in the code, so yes OSGi services would be of course better to consume (and one can already do so e.g. with IWorkspace). But there are still a lot of code that boils down to static access. So the main goal for me is to make ResourcesPlugin be loadable without DataArea available. Also several calls to IWorkspace itself can be delayed, eg addResourceChangeListener to a later point.
(In reply to Christoph Laeubrich from comment #3) > @Andrey the main problem is that referencing ANY class from the > "ResourcesPlugin" packages currently can cause the issue even though > ResourcesPlugin itself is never called in the code I see. The problem here is that resources bundle is all about workspace and workspace without data area initialized doesn't make much sense. But if you have concrete improvements in mind, just push gerrits to continue discussion.
We can remove the code from the ResourcePlugin (Activator), in which case loading a class won't trigger the Activator code. However, there are many plugins which depend on the ResourcePlugin bundle with the assumption that when the ResourcePlugin is available that calls to the getStateLocation will work. That's the real problem. I published a whiteboard for IResourceChangeListener that allows downstream plugins to register an IResourceChangeListener class that is automatically picked up by the workbench. The problem is that many of the uses of registering their listeners are in imperative code because they aren't DS. We aren't going to solve it from a bottom up perspective; we'll need to solve it top-down.
