Community
Participate
Working Groups
I got the following exception once when attempting to get the undo history from a source viewer in my code. Looking at the implementation of this method, I cannot seem to see how this could occur since all access to this list in DefaultOperationHistory are synchronized. Maybe it is the creation of the iterator outside of the synchronized block? Note that I have been unable to reproduce this since I first saw it since it seems to be a rare race condition. Exception Stack Trace: java.util.ConcurrentModificationException at java.util.AbstractList$Itr.checkForComodification(AbstractList.java:372) at java.util.AbstractList$Itr.next(AbstractList.java:343) at org.eclipse.core.commands.operations.DefaultOperationHistory.filter(DefaultOperationHistory.java:558) at org.eclipse.core.commands.operations.DefaultOperationHistory.getUndoHistory(DefaultOperationHistory.java:843)
Created attachment 177375 [details] patch Attached is a patch that should fix this problem. The issue is that the iterator needs to be created inside of the synchronized block since it stores the modcount when it is created, which is the field that is used to determine if the was concurrent modification.
*** Bug 330258 has been marked as a duplicate of this bug. ***
I'm a little worried because the comments in bug 238397 suggest that this problem should have been fixed in that bug. However, this exception has always been difficult to reproduce/force. Given there have been two reports of the problem since the fix for bug 238397, I released the patch. I verified that our test cases all still pass (including the multi-thread stress tests), though I don't have a way to force the modification exception and then prove that this fixes it. All that said, I don't see harm in this fix and have released to HEAD >20101216.
Hi Susan. Just to ease your worries :) From what I can see in the patches and comments in bug 238397, they properly synchronized write access to the history. In those cases the synchronization gap in filter() is indeed no problem because the lock has already been acquired by all callers. However, those patches apparently missed the read access cases through getUndoHistory() and getRedoHistory() that call filter() without first acquiring a lock, thus opening the gap for this ConcurrentModificationException to happen.
marking this verified I20110124-1800 - the code change is in the build - the tests are all passing but as stated above, we don't have a case that reliably reproduces this problem in order to verify it for sure.
