Bug 238397 [Commands] Concurrent modification possible in DefaultOperationHistory
Build ID: 3.3.3.r33x_r20080129 Steps To Reproduce: This is kind of hard to reproduce, nevertheless the systematic gap can be seen directly within the coding. Please have a look into the coding of the method flushUndo(...) in org.eclipse.core.commands.operations.DefaultOperationHistory. The undoList.remove is triggered without any synchronization (in contrast to the other coding locations the list is modified). This may lead to ConcurrentModificationExceptions in case of very rapidly fired Undo/redo events (in fact that's what happended within one of our automated tests). Furthermore, the method filter(...) called right before the for-loop first gets an iterator for the list, then aquires the lock an then loops over the iterator. In rare cases this may lead to IndexOutOfBoundsExceptions when another thread modified the list in between getting the iterator and looping over it (actually also this happened in the mentioned test case). From my point of view the lock should already by aquired in the flushUndo before calling the filter(...) method. More information: I'm sorry for not being able to offer a reproducable code snippet, since our case we spotted this needs a quite complicated setup, a lot of testdata and used quite a lot of parallel threading.
bundles/org.eclipse.core.commands/src/org/eclipse/core/commands/operations/DefaultOperationHistory.java
* Implementations for the direct undo and redo of a specified operation are
 * available. If a strict linear undo is to be enforced, than an
 * IOperationApprover should be installed that prevents undo and redo of any
 * operation that is not the most recently undone or redone operation in all of
 * its undo contexts.
 * </p>
 * <p>
 * The data structures used by the DefaultOperationHistory are synchronized, and
 * entry points that modify the undo and redo history concurrently are also
 * synchronized. This means that the DefaultOperationHistory is relatively
 * "thread-friendly" in its implementation. Outbound notifications or operation
 * approval requests will occur on the thread that initiated the request.
 * Clients may use DefaultOperationHistory API from any thread; however,
 * listeners or operation approvers that receive notifications from the
 * DefaultOperationHistory must be prepared to receive these notifications from
 * a background thread. Any UI access occurring inside these notifications must
 * be properly synchronized using the techniques specified by the client's
 * widget library.
 * </p>
 * 
 * <p>
 * This implementation is not intended to be subclassed.
 * </p>
 * 
 * @see org.eclipse.core.commands.operations.IOperationHistory
 * @see org.eclipse.core.commands.operations.IOperationApprover
 * 
 * @since 3.1
 */
public final class DefaultOperationHistory implements IOperationHistory {