Bugzilla – Full Text Bug Listing
| Summary: | [EditorMgmt] AFE in SaveablesList.decrementRefCount when closing editor | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Markus Keller <markus.kell.r> | ||||
| Component: | UI | Assignee: | Andrey Loskutov <loskutov> | ||||
| Status: | RESOLVED FIXED | QA Contact: | |||||
| Severity: | major | ||||||
| Priority: | P3 | CC: | birdwatcher1024, daniel_megert, dh_tue, ed, error-reports-inbox, gautier.desaintmartinlacaze, h.klene, karenfbutzke, lbarbareau, loskutov, martin.skorsky, mauromol, Olivier_Thomann, psuzzi, pwebster, remy.suen, tomasz.zarna | ||||
| Version: | 3.8 | ||||||
| Target Milestone: | 4.8 M4 | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| See Also: |
https://bugs.eclipse.org/bugs/show_bug.cgi?id=353692 https://git.eclipse.org/r/110776 https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=39692a9129b708805062e8d4c6ebbdb60a71e16f |
||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
|
Description
Markus Keller
Markus, was wondering why you added the extra if statement? Couldn't you have just used the isTrue(boolean, String) method as is? http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=4305a9e8fbe6c1b6971a4bd1e4e4525885999635 > Markus, was wondering why you added the extra if statement? Couldn't you have
> just used the isTrue(boolean, String) method as is?
The problem with Assert#isTrue(boolean, String) is that the second argument is always evaluated. I didn't want to risk any performance problems or even a ClassCastException if I got something wrong and the key is not a Saveable in certain cases.
Also in my log file ... cannot remember anything special Version: 4.2.0 Build id: I20120608-1400 I think I'm getting the same exception or something very similar. It happens when I have one file open in two different compare editors, and if I close the first compare editor before the second. Can reproduce as follows: 1) Compare two files with each other to open a compare editor. 2) Compare one file from step 1 with a third file. Now one file is open in two different compare errors at once. 3) Close the compare editor opened in step 1. 4) Try to close the compare editor opened in step 2. The editor won't close and an error log entry for the AssertionFailedException appears. However, no exceptions occur when I close the second compare editor before closing the first. Reproduced in Eclipse 3.7.2 (build ID 20120216-1857). Steps of comment 4 also apply to: Version: 4.3.0M1 Build id: I20120810-1300 Created attachment 222837 [details]
exception stack trace
I get this with some frequency when doing a 'Close All Editors'. I have no compare editors open. The Close All fails to close any editors, so I repeat the action and it works the second time. I have not found any rhyme or reason for seeing this problem.
I am currently using build 4.2.2-M20121024-1600 with Egit 2.1 installed
I'm bumping the severity to normal since it is annoying to have the Close All action fail. *** Bug 407400 has been marked as a duplicate of this bug. *** *** Bug 393329 has been marked as a duplicate of this bug. *** I'm seeing this in 4.5M1. Since the workaround is just to Close All Editors twice, perhaps part of a fix could be to catch the exception more narrowly so that it doesn't cause the close all loop to fail. *** Bug 461230 has been marked as a duplicate of this bug. *** *** Bug 451235 has been marked as a duplicate of this bug. *** Mars. Raising to major since this can occur silently when auto-saving before a launch. Consequently you can waste time debugging stale code. *** Bug 478596 has been marked as a duplicate of this bug. *** Also present in: Version: Oxygen Release (4.7.0) Build id: 20170620-1800 Still in 4.8. See also Aeri query: https://dev.eclipse.org/recommenders/committers/aeri/v2/#!/problems/54c4f061bee810030da07fff Steps to reproduce: Open history view on some git project. In any commit, right click one java file and say "compare with working tree". Compare editor opens. On the same file in the history view, say "open working tree version". Java editor opens. Right click on the editor tab and say "close all". KABOOM. Stack trace from 4.8: org.eclipse.core.runtime.AssertionFailedException: assertion failed: org.eclipse.ui.texteditor.AbstractTextEditor$TextEditorSavable@3943eb67: BasicModule.java at org.eclipse.core.runtime.Assert.isTrue(Assert.java:110) at org.eclipse.ui.internal.SaveablesList.decrementRefCount(SaveablesList.java:155) at org.eclipse.ui.internal.SaveablesList.removeModel(SaveablesList.java:174) at org.eclipse.ui.internal.SaveablesList.postClose(SaveablesList.java:702) at org.eclipse.ui.internal.WorkbenchPage.firePartClosed(WorkbenchPage.java:5190) at org.eclipse.ui.internal.e4.compatibility.CompatibilityPart.lambda$0(CompatibilityPart.java:104) at org.eclipse.e4.ui.services.internal.events.UIEventHandler$1.run(UIEventHandler.java:40) at org.eclipse.swt.widgets.Synchronizer.syncExec(Synchronizer.java:233) at org.eclipse.ui.internal.UISynchronizer.syncExec(UISynchronizer.java:144) at org.eclipse.swt.widgets.Display.syncExec(Display.java:4889) at org.eclipse.e4.ui.internal.workbench.swt.E4Application$1.syncExec(E4Application.java:212) at org.eclipse.e4.ui.services.internal.events.UIEventHandler.handleEvent(UIEventHandler.java:36) at org.eclipse.equinox.internal.event.EventHandlerWrapper.handleEvent(EventHandlerWrapper.java:201) at org.eclipse.equinox.internal.event.EventHandlerTracker.dispatchEvent(EventHandlerTracker.java:196) at org.eclipse.equinox.internal.event.EventHandlerTracker.dispatchEvent(EventHandlerTracker.java:1) at org.eclipse.osgi.framework.eventmgr.EventManager.dispatchEvent(EventManager.java:230) at org.eclipse.osgi.framework.eventmgr.ListenerQueue.dispatchEventSynchronous(ListenerQueue.java:148) at org.eclipse.equinox.internal.event.EventAdminImpl.dispatchEvent(EventAdminImpl.java:135) at org.eclipse.equinox.internal.event.EventAdminImpl.sendEvent(EventAdminImpl.java:78) at org.eclipse.equinox.internal.event.EventComponent.sendEvent(EventComponent.java:39) at org.eclipse.e4.ui.services.internal.events.EventBroker.send(EventBroker.java:52) at org.eclipse.e4.ui.internal.workbench.UIEventPublisher.notifyChanged(UIEventPublisher.java:60) at org.eclipse.emf.common.notify.impl.BasicNotifierImpl.eNotify(BasicNotifierImpl.java:374) at org.eclipse.e4.ui.model.application.ui.impl.UIElementImpl.setWidget(UIElementImpl.java:261) at org.eclipse.e4.ui.workbench.renderers.swt.SWTPartRenderer.unbindWidget(SWTPartRenderer.java:146) at org.eclipse.e4.ui.workbench.renderers.swt.SWTPartRenderer.disposeWidget(SWTPartRenderer.java:169) at org.eclipse.e4.ui.workbench.renderers.swt.ContributedPartRenderer.disposeWidget(ContributedPartRenderer.java:270) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.safeRemoveGui(PartRenderingEngine.java:935) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.access$1(PartRenderingEngine.java:863) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$4.run(PartRenderingEngine.java:858) at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.removeGui(PartRenderingEngine.java:842) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.subscribeTopicToBeRendered(PartRenderingEngine.java:180) at sun.reflect.GeneratedMethodAccessor57.invoke(Unknown Source) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:498) at org.eclipse.e4.core.internal.di.MethodRequestor.execute(MethodRequestor.java:55) at org.eclipse.e4.core.di.internal.extensions.EventObjectSupplier$DIEventHandler.handleEvent(EventObjectSupplier.java:88) at org.eclipse.equinox.internal.event.EventHandlerWrapper.handleEvent(EventHandlerWrapper.java:201) at org.eclipse.equinox.internal.event.EventHandlerTracker.dispatchEvent(EventHandlerTracker.java:196) at org.eclipse.equinox.internal.event.EventHandlerTracker.dispatchEvent(EventHandlerTracker.java:1) at org.eclipse.osgi.framework.eventmgr.EventManager.dispatchEvent(EventManager.java:230) at org.eclipse.osgi.framework.eventmgr.ListenerQueue.dispatchEventSynchronous(ListenerQueue.java:148) at org.eclipse.equinox.internal.event.EventAdminImpl.dispatchEvent(EventAdminImpl.java:135) at org.eclipse.equinox.internal.event.EventAdminImpl.sendEvent(EventAdminImpl.java:78) at org.eclipse.equinox.internal.event.EventComponent.sendEvent(EventComponent.java:39) at org.eclipse.e4.ui.services.internal.events.EventBroker.send(EventBroker.java:52) at org.eclipse.e4.ui.internal.workbench.UIEventPublisher.notifyChanged(UIEventPublisher.java:60) at org.eclipse.emf.common.notify.impl.BasicNotifierImpl.eNotify(BasicNotifierImpl.java:374) at org.eclipse.e4.ui.model.application.ui.impl.UIElementImpl.setToBeRendered(UIElementImpl.java:303) at org.eclipse.e4.ui.internal.workbench.PartServiceImpl.hidePart(PartServiceImpl.java:1354) at org.eclipse.e4.ui.internal.workbench.PartServiceImpl.hidePart(PartServiceImpl.java:1284) at org.eclipse.e4.ui.workbench.renderers.swt.StackRenderer.closeSiblingParts(StackRenderer.java:1650) at org.eclipse.e4.ui.workbench.renderers.swt.StackRenderer.closeSiblingParts(StackRenderer.java:1607) at org.eclipse.e4.ui.workbench.renderers.swt.StackRenderer.access$4(StackRenderer.java:1601) at org.eclipse.e4.ui.workbench.renderers.swt.StackRenderer$11.widgetSelected(StackRenderer.java:1499) at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:249) SaveablesList.incrementRefCount(Map<Saveable, Integer>, Saveable) increments refcount for org.eclipse.egit.ui.internal.revision.GitCompareFileRevisionEditorInput$InternalResourceSaveableComparison even if the org.eclipse.ui.texteditor.AbstractTextEditor$TextEditorSavable is the key, because both implements Saveable.equals() which allows different types to be equal if they represent "same" "shared dirty state". Now the SaveablesList.modelRefCounts contains InternalResourceSaveableComparison as the key and "2" as value. This would be OK, but ... the equals() implementation of that keys depends on the state of the objects! Means, once the InternalResourceSaveableComparison is disposed, it is not equal to anything else except itself anymore, and so the attempt to retrieve the refcount for the TextEditorSavable in SaveablesList.decrementRefCount fails. The point is: SaveablesList implementation depends on the equals() implementation of the Saveables which should NOT be dependent on the object states, but exact the opposite is implemented in LocalResourceSaveableComparison, where the dispose() breaks equals(), as a side effect of fix for bug 353692. New Gerrit change created: https://git.eclipse.org/r/110776 (In reply to Eclipse Genie from comment #18) > New Gerrit change created: https://git.eclipse.org/r/110776 The idea is to allow "broken" equals() implementations by tracking all "equal" saveables. This patch seem to work but needs tests, therefore "WIP". Gerrit change https://git.eclipse.org/r/110776 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=39692a9129b708805062e8d4c6ebbdb60a71e16f SaveablesList can work now even if the Saveable.equals() is not properly implemented and changes the behavior during the life cycle of the Saveable. |