Bug 558510 - ConcurrentModificationException when editing breakpoint condition, no content assist afterwards
Summary: ConcurrentModificationException when editing breakpoint condition, no content...
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Text (show other bugs)
Version: 4.15   Edit
Hardware: All All
: P3 major (vote)
Target Milestone: 4.15 M1   Edit
Assignee: Carsten Hammer CLA
QA Contact:
URL:
Whiteboard:
Keywords: regression
Depends on:
Blocks:
 
Reported: 2019-12-20 09:18 EST by Simeon Andreev CLA
Modified: 2020-01-07 07:13 EST (History)
5 users (show)

See Also:


Attachments
Video showing the problem. (2.03 MB, video/mp4)
2019-12-20 09:18 EST, Simeon Andreev CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Simeon Andreev CLA 2019-12-20 09:18:34 EST
Created attachment 281289 [details]
Video showing the problem.

I think this is caused by changes for bug 548309. Its not directly a regression, but with the for-loop added for bug 548309 the concurrent modification becomes visible.

https://git.eclipse.org/r/#/c/152786/4/org.eclipse.jface.text/src/org/eclipse/jface/text/TextViewer.java

Previously, the code just accessed whatever the underlying array was at index N. Now Java throws the exception.

I see this *a lot*, its difficult to edit a breakpoint condition as content assist breaks all the time (and I have to re-open the dialog).


java.util.ConcurrentModificationException
	at java.util.ArrayList$Itr.checkForComodification(ArrayList.java:909)
	at java.util.ArrayList$Itr.next(ArrayList.java:859)
	at org.eclipse.jface.text.TextViewer.fireInputDocumentAboutToBeChanged(TextViewer.java:2748)
	at org.eclipse.jface.text.TextViewer.setDocument(TextViewer.java:2796)
	at org.eclipse.jface.text.source.SourceViewer.setDocument(SourceViewer.java:663)
	at org.eclipse.jface.text.source.SourceViewer.setDocument(SourceViewer.java:603)
	at org.eclipse.jface.text.TextViewer.handleDispose(TextViewer.java:1809)
	at org.eclipse.jface.text.source.SourceViewer.handleDispose(SourceViewer.java:791)
	at org.eclipse.jface.text.TextViewer.lambda$1(TextViewer.java:1723)
	at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:127)
	at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:89)
	at org.eclipse.swt.widgets.Display.sendEvent(Display.java:5741)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1423)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1449)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1432)
	at org.eclipse.swt.widgets.Widget.notifyListeners(Widget.java:1221)
	at org.eclipse.swt.custom.StyledText.handleDispose(StyledText.java:5980)
	at org.eclipse.swt.custom.StyledText.lambda$1(StyledText.java:5792)
	at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:89)
	at org.eclipse.swt.widgets.Display.sendEvent(Display.java:5741)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1423)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1449)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1428)
	at org.eclipse.swt.widgets.Widget.release(Widget.java:1240)
	at org.eclipse.swt.widgets.Control.release(Control.java:4578)
	at org.eclipse.swt.widgets.Composite.releaseChildren(Composite.java:1502)
	at org.eclipse.swt.widgets.Widget.release(Widget.java:1243)
	at org.eclipse.swt.widgets.Control.release(Control.java:4578)
	at org.eclipse.swt.widgets.Composite.releaseChildren(Composite.java:1502)
	at org.eclipse.swt.widgets.Widget.release(Widget.java:1243)
	at org.eclipse.swt.widgets.Control.release(Control.java:4578)
	at org.eclipse.swt.widgets.Composite.releaseChildren(Composite.java:1502)
	at org.eclipse.swt.widgets.Widget.release(Widget.java:1243)
	at org.eclipse.swt.widgets.Control.release(Control.java:4578)
	at org.eclipse.swt.widgets.Composite.releaseChildren(Composite.java:1502)
	at org.eclipse.swt.widgets.Widget.release(Widget.java:1243)
	at org.eclipse.swt.widgets.Control.release(Control.java:4578)
	at org.eclipse.swt.widgets.Composite.releaseChildren(Composite.java:1502)
	at org.eclipse.swt.widgets.Widget.release(Widget.java:1243)
	at org.eclipse.swt.widgets.Control.release(Control.java:4578)
	at org.eclipse.swt.widgets.Composite.releaseChildren(Composite.java:1502)
	at org.eclipse.swt.widgets.Widget.release(Widget.java:1243)
	at org.eclipse.swt.widgets.Control.release(Control.java:4578)
	at org.eclipse.swt.widgets.Composite.releaseChildren(Composite.java:1502)
	at org.eclipse.swt.widgets.Widget.release(Widget.java:1243)
	at org.eclipse.swt.widgets.Control.release(Control.java:4578)
	at org.eclipse.swt.widgets.Composite.releaseChildren(Composite.java:1502)
	at org.eclipse.swt.widgets.Widget.release(Widget.java:1243)
	at org.eclipse.swt.widgets.Control.release(Control.java:4578)
	at org.eclipse.swt.widgets.Composite.releaseChildren(Composite.java:1502)
	at org.eclipse.swt.widgets.Widget.release(Widget.java:1243)
	at org.eclipse.swt.widgets.Control.release(Control.java:4578)
	at org.eclipse.swt.widgets.Composite.releaseChildren(Composite.java:1502)
	at org.eclipse.swt.widgets.Canvas.releaseChildren(Canvas.java:279)
	at org.eclipse.swt.widgets.Decorations.releaseChildren(Decorations.java:486)
	at org.eclipse.swt.widgets.Shell.releaseChildren(Shell.java:3223)
	at org.eclipse.swt.widgets.Widget.release(Widget.java:1243)
	at org.eclipse.swt.widgets.Control.release(Control.java:4578)
	at org.eclipse.swt.widgets.Widget.dispose(Widget.java:529)
	at org.eclipse.swt.widgets.Shell.dispose(Shell.java:3146)
	at org.eclipse.jface.window.Window.close(Window.java:335)
	at org.eclipse.jface.dialogs.Dialog.close(Dialog.java:988)
	at org.eclipse.jface.preference.PreferenceDialog.close(PreferenceDialog.java:305)
	at org.eclipse.ui.internal.dialogs.FilteredPreferenceDialog.close(FilteredPreferenceDialog.java:651)
	at org.eclipse.jface.preference.PreferenceDialog.cancelPressed(PreferenceDialog.java:272)
	at org.eclipse.jface.preference.PreferenceDialog.buttonPressed(PreferenceDialog.java:237)
	at org.eclipse.jface.dialogs.Dialog.lambda$0(Dialog.java:619)
	at org.eclipse.swt.events.SelectionListener$1.widgetSelected(SelectionListener.java:84)
	at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:252)
	at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:89)
	at org.eclipse.swt.widgets.Display.sendEvent(Display.java:5741)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1423)
	at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:5000)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:4493)
	at org.eclipse.jface.window.Window.runEventLoop(Window.java:823)
	at org.eclipse.jface.window.Window.open(Window.java:799)
	at org.eclipse.ui.dialogs.PropertyDialogAction.run(PropertyDialogAction.java:153)
	at org.eclipse.jdt.internal.debug.ui.actions.JavaBreakpointPropertiesRulerAction.run(JavaBreakpointPropertiesRulerAction.java:70)
	at org.eclipse.jface.action.Action.runWithEvent(Action.java:474)
	at org.eclipse.ui.texteditor.AbstractRulerActionDelegate.runWithEvent(AbstractRulerActionDelegate.java:112)
	at org.eclipse.ui.internal.PluginAction.runWithEvent(PluginAction.java:229)
	at org.eclipse.jface.action.ActionContributionItem.handleWidgetSelection(ActionContributionItem.java:579)
	at org.eclipse.jface.action.ActionContributionItem.lambda$4(ActionContributionItem.java:413)
	at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:89)
	at org.eclipse.swt.widgets.Display.sendEvent(Display.java:5741)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1423)
	at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:5000)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:4493)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$5.run(PartRenderingEngine.java:1160)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:338)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1049)
	at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:155)
	at org.eclipse.ui.internal.Workbench.lambda$3(Workbench.java:660)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:338)
	at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:559)
	at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:154)
	at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:150)
	at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:203)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:137)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:107)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:401)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:255)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:657)
	at org.eclipse.equinox.launcher.Main.basicRun(Main.java:594)
	at org.eclipse.equinox.launcher.Main.run(Main.java:1465)
	at org.eclipse.equinox.launcher.Main.main(Main.java:1438)

Build is:

Eclipse SDK
Version: 2020-03 (4.15)
Build id: I20191219-1800
Comment 1 Simeon Andreev CLA 2019-12-20 09:41:24 EST
Probably the race condition should be fixed, but I think reverting the change from bug 548309 is faster.
Comment 2 Dani Megert CLA 2019-12-20 09:49:18 EST
https://git.eclipse.org/r/#/c/152786/ should be reverted asap and each change verified that it does not break things.
Comment 3 Eclipse Genie CLA 2019-12-20 10:15:02 EST
New Gerrit change created: https://git.eclipse.org/r/154893
Comment 4 Carsten Hammer CLA 2019-12-20 10:17:15 EST
Sorry, I did not see your communication while preparing a gerrit..
Comment 5 Andrey Loskutov CLA 2019-12-20 11:08:09 EST
Looking on the optimization made we see that the old code sometimes iterated over listeners without copying them first, but sometimes with a copy. 

The simplest fix (after reverting optimization back to original state) would be to iterate over a copy of listener lists.

Carsten, please provide first a simple revert of the "for each" optimization for TextViewer.

Second, please go over all "for each" optimization commits and check if any of them did similar changes like here (iteration over listeners not copied before) , and also revert them too.

Third, create a bug describing the problem with listeners in TextViewer (and other cases if there any).
Comment 6 Andrey Loskutov CLA 2019-12-20 11:21:19 EST
See also bug 558339 comment 4. Same problem.
Comment 7 Carsten Hammer CLA 2019-12-20 11:42:13 EST
It looks like my rights are too limited to use the revert button in gerrit. I thought it just creates a new gerrit with the reversion. That is strange - I have the right to create a gerrit..
Comment 8 Eclipse Genie CLA 2019-12-20 12:04:22 EST
New Gerrit change created: https://git.eclipse.org/r/154908
Comment 9 Carsten Hammer CLA 2019-12-22 03:58:38 EST
(In reply to Andrey Loskutov from comment #5)
> Looking on the optimization made we see that the old code sometimes iterated
> over listeners without copying them first, but sometimes with a copy. 
> 
> The simplest fix (after reverting optimization back to original state) would
> be to iterate over a copy of listener lists.
Please allow me to stress the fact that this is no fix, imho. It just hides the problem as java has no chance to detect problems in concurrent processing then.

> 
> Carsten, please provide first a simple revert of the "for each" optimization
> for TextViewer.

done, see https://git.eclipse.org/r/#/c/154908/

> 
> Second, please go over all "for each" optimization commits and check if any
> of them did similar changes like here (iteration over listeners not copied
> before) , and also revert them too.
Just checked the one where the TextViewer class was part of. 

> 
> Third, create a bug describing the problem with listeners in TextViewer (and
> other cases if there any).

bug 558557
feel free to modify the description if that is not reflecting your point of view.
Comment 10 Simeon Andreev CLA 2019-12-31 03:38:58 EST
On a side note, can also occur when using the Java editor. Closing and opening the editor restores content assist functionality.
Comment 12 Andrey Loskutov CLA 2020-01-07 07:13:05 EST
Verified in I20200106-1805