Summary: | Don't attempt to save preferences at shutdown | ||
---|---|---|---|
Product: | [Eclipse Project] Platform | Reporter: | Alex Blewitt <alex.blewitt> |
Component: | Resources | Assignee: | Platform-Resources-Inbox <platform-resources-inbox> |
Status: | NEW --- | QA Contact: | |
Severity: | normal | ||
Priority: | P3 | CC: | loskutov |
Version: | 4.16 | ||
Target Milestone: | --- | ||
Hardware: | All | ||
OS: | All | ||
Whiteboard: |
Description
Alex Blewitt
2020-05-30 20:42:55 EDT
The fix probably looks something like: diff --git bundles/org.eclipse.core.resources/src/org/eclipse/core/resources/ResourcesPlugin.java bundles/org.eclipse.core.resources/src/org/eclipse/core/resources/ResourcesPlugin.java index 636b3a8c8..56770b27a 100644 --- bundles/org.eclipse.core.resources/src/org/eclipse/core/resources/ResourcesPlugin.java +++ bundles/org.eclipse.core.resources/src/org/eclipse/core/resources/ResourcesPlugin.java @@ -454,8 +454,6 @@ public final class ResourcesPlugin extends Plugin { if (workspaceRegistration != null) { workspaceRegistration.unregister(); } - // save the preferences for this plug-in - getPlugin().savePluginPreferences(); workspace.close(null); // Forget workspace only if successfully closed, to And for the AbstractUIPlugin diff --git bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/plugin/AbstractUIPlugin.java bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/plugin/AbstractUIPlugin.java index 087c67f829..022b509549 100644 --- bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/plugin/AbstractUIPlugin.java +++ bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/plugin/AbstractUIPlugin.java @@ -679,7 +679,6 @@ public abstract class AbstractUIPlugin extends Plugin { context.removeBundleListener(bundleListener); } saveDialogSettings(); - savePreferenceStore(); preferenceStore = null; if (imageRegistry != null) imageRegistry.dispose(); Alternatively the savePreference could be modified such that it handles the possibility of an assertion failure here: @Deprecated public final void savePluginPreferences() { Location instance = InternalPlatform.getDefault().getInstanceLocation(); if (instance == null || !instance.isSet()) // If the instance area is not set there is no point in getting or setting the preferences. // There is nothing to save in this case. return; We could catch the exception generated from the internalPlatform, and if it's not running, return early. Thoughts? (In reply to Alex Blewitt from comment #2) > Alternatively the savePreference could be modified such that it handles the > possibility of an assertion failure here: > > @Deprecated > public final void savePluginPreferences() { > Location instance = InternalPlatform.getDefault().getInstanceLocation(); > if (instance == null || !instance.isSet()) > // If the instance area is not set there is no point in getting or > setting the preferences. > // There is nothing to save in this case. > return; > > We could catch the exception generated from the internalPlatform, and if > it's not running, return early. > > Thoughts? what says if(! Platform.isRunning()) ? I would like to understand how can you reproduce the problem - it should happen every day with lot of users, but we haven't seen such reports yet. Or is this just theoretical discussion "what if..."? (In reply to Andrey Loskutov from comment #3) > (In reply to Alex Blewitt from comment #2) > > public final void savePluginPreferences() { > > Location instance = InternalPlatform.getDefault().getInstanceLocation(); > > what says if(! Platform.isRunning()) ? The call to InternalPlatform.getDefault().getInstanceLocation() is where the assert happens. https://github.com/eclipse/eclipse.platform.runtime/blob/410c981210bdeb2a2c465cd47ff5a5e58e208be8/bundles/org.eclipse.core.runtime/src/org/eclipse/core/internal/runtime/InternalPlatform.java#L330-L333 My point is the code is defensively written such that if the instance location returns null then it leaves early, but the contract of getInstanceLocation() doesn't return null, it throws an assertion error in this case. > I would like to understand how can you reproduce the problem - it should > happen every day with lot of users, but we haven't seen such reports yet. Or > is this just theoretical discussion "what if..."? I managed to produce it when testing various scenarios, so I don't think it's common. It's really a stop-ordering problem (which is the inverse of a start-ordering problem). If you stop platform before you stop org.eclipse.common.runtime, and then you stop a bundle that descends from Plugin and has an activator, you'll see this assertion error happening. I don't think you'll see this generally -- or if you do, the log entry will be swallowed because you are shutting down Eclipse, and by that point, the platform.log has gone away anyway. |