search

eclipse-platform / eclipse.platform.resources

Eclipse Public License 2.0
3 stars 18 forks source link

RefreshManager.manageAutoRefresh calls ResourcesPlugin.getWorkspace before the Workspace is fully open #97

Closed laeubi closed 2 years ago

laeubi commented 2 years ago

Just another one...

java.lang.IllegalStateException: Workspace is already closed or not ready yet. Consider tracking the org.eclipse.core.resources.IWorkspace service (using your favorite technique, e.g. Declarative Services, ServiceTracker, Blueprint, ...) instead of calling the static method here to prevent such issues!
    at org.eclipse.core.resources.ResourcesPlugin.getWorkspace(ResourcesPlugin.java:423)
    at org.eclipse.core.internal.refresh.PollingMonitor.runOnce(PollingMonitor.java:173)
    at org.eclipse.core.internal.refresh.MonitorManager.start(MonitorManager.java:291)
    at org.eclipse.core.internal.refresh.RefreshManager.manageAutoRefresh(RefreshManager.java:59)
    at org.eclipse.core.internal.refresh.RefreshManager.startup(RefreshManager.java:125)
    at org.eclipse.core.internal.resources.Workspace.startup(Workspace.java:2547)
    at org.eclipse.core.internal.resources.Workspace.open(Workspace.java:2241)
mickaelistria commented 2 years ago

I'm curious about those issues: do I get it right they were not existing earlier, in which case the behavior is a regression? In such case, I don't think that an exception should be thrown but activation should take place when invoking ResourcesPlugin.getWorkspace() anyway. We can maybe add some warning to help migrating to better approaches, but the risk of breaking existing code is high if getWorkspace() starts sending exception when it was not before.

laeubi commented 2 years ago

do I get it right they were not existing earlier, in which case the behavior is a regression?

The stack trace is when I (locally) enable a much more strict checking of workspace access. See this code lines:

https://github.com/eclipse-platform/eclipse.platform.resources/blob/684cfd44f038d76465a3dacd99e204ea9bee7c97/bundles/org.eclipse.core.resources/src/org/eclipse/core/resources/ResourcesPlugin.java#L474-L477

they claim that

Remember workspace before opening, to make it easier to debug cases where open() is failing.

but actually this is not true! if you not assign it to the static field before open the workspace, then it simply fails (as with a stack similar to shown here) because while open is running, code access ResourcesPlugin.getWorkspace() even though the open is currently in progress.

laeubi commented 2 years ago

I don't think that an exception should be thrown but activation should take place when invoking

The problem is not that activation is taking place, but that at that time the Workspace might not be available (e.g. Workspace Selection Dialog is shown) so that exception is correct here.

mickaelistria commented 2 years ago

The stack trace is when I (locally) enable a much more strict checking of workspace access.

OK, perfect.

so that exception is correct here.

As long as this exception is not shown in cases that used to work fine, it's all good.