Comment 1 Remy Suen 2012-03-09 16:26:16 EST

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

Comment 2 Markus Keller 2012-03-11 13:56:00 EDT

> 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.

Comment 3 Holger Klene 2012-07-11 18:28:17 EDT

Also in my log file ... cannot remember anything special
Version: 4.2.0 Build id: I20120608-1400

Comment 4 Laura C 2012-08-07 13:00:49 EDT

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).

Comment 5 Holger Klene 2012-08-26 08:58:14 EDT

Steps of comment 4 also apply to:
Version: 4.3.0M1 Build id: I20120810-1300

Comment 6 Karen Butzke 2012-10-25 12:12:44 EDT

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

Comment 7 Karen Butzke 2012-10-25 12:13:55 EDT

I'm bumping the severity to normal since it is annoying to have the Close All action fail.

Comment 8 Dani Megert 2013-05-08 06:30:49 EDT

*** Bug 407400 has been marked as a duplicate of this bug. ***

Comment 9 Dani Megert 2013-08-23 05:00:01 EDT

*** Bug 393329 has been marked as a duplicate of this bug. ***

Comment 10 Ed Willink 2014-09-22 10:42:11 EDT

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.

Comment 11 Markus Keller 2015-05-29 06:19:05 EDT

*** Bug 461230 has been marked as a duplicate of this bug. ***

Comment 12 Markus Keller 2015-05-29 06:19:39 EDT

*** Bug 451235 has been marked as a duplicate of this bug. ***

Comment 13 Ed Willink 2015-06-27 13:55:35 EDT

Mars. Raising to major since this can occur silently when auto-saving before a launch. Consequently you can waste time debugging stale code.

Comment 14 Markus Keller 2017-06-28 05:37:51 EDT

*** Bug 478596 has been marked as a duplicate of this bug. ***

Comment 15 Dennis Hendriks 2017-09-13 04:51:02 EDT

Also present in:
Version: Oxygen Release (4.7.0)
Build id: 20170620-1800

Comment 16 Andrey Loskutov 2017-10-28 06:09:31 EDT

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)

Comment 17 Andrey Loskutov 2017-10-28 07:41:25 EDT

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.

Comment 18 Eclipse Genie 2017-10-30 18:00:33 EDT

New Gerrit change created: https://git.eclipse.org/r/110776

Comment 19 Andrey Loskutov 2017-10-30 18:06:54 EDT

(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".

Comment 20 Eclipse Genie 2017-11-17 05:03:40 EST

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

Comment 21 Andrey Loskutov 2017-11-20 03:18:28 EST

SaveablesList can work now even if the Saveable.equals() is not properly implemented and changes the behavior during the life cycle of the Saveable.