Bug 93192 Operation history MUST make sure that dispose is called whenever a undo/redo object is removed from the histroy
To avoid leaking change object it is absolutely important that we always dispose undo/redo object when they are removed or replace in the history. The reproduce the problem apply the attached patch to LTK core to simpulate an unvalid operation. Then do the following: - open an editor - execute an extract method operation - execute undo observe that dispose isn't called on the refactoring undo object. The problem is that field triggeringOperation in TriggeredOperations is already null when dispose is called on TriggeredOperations. The triggeredOperation is set to null by the followin stack trace Thread [main] (Suspended (breakpoint at line 84 in TriggeredOperations)) TriggeredOperations.remove(IUndoableOperation) line: 84 TriggeredOperations.removeContext(IUndoContext) line: 110 DefaultOperationHistory.flushUndo(IUndoContext) line: 544 DefaultOperationHistory.dispose(IUndoContext, boolean, boolean, boolean) line: 258 UndoActionHandler.flush() line: 49 OperationHistoryActionHandler$2.run() line: 275 RunnableLock.run() line: 35 UISynchronizer(Synchronizer).runAsyncMessages(boolean) line: 118 Display.runAsyncMessages(boolean) line: 2877 Display.readAndDispatch() line: 2536 Workbench.runEventLoop(Window$IExceptionHandler, Display) line: 1569 Workbench.runUI() line: 1533 Workbench.createAndRunWorkbench(Display, WorkbenchAdvisor) line: 306 PlatformUI.createAndRunWorkbench(Display, WorkbenchAdvisor) line: 143 IDEApplication.run(Object) line: 103 PlatformActivator$1.run(Object) line: 228 EclipseStarter.run(Object) line: 345 EclipseStarter.run(String[], Runnable) line: 158 NativeMethodAccessorImpl.invoke0(Method, Object, Object[]) line: not available [native method] NativeMethodAccessorImpl.invoke(Object, Object[]) line: 85 NativeMethodAccessorImpl.invoke(Method, Object, Object[]) line: 58 DelegatingMethodAccessorImpl.invoke(Method, Object, Object[]) line: 60 Method.invoke(Object, Object[]) line: 391 Main.invokeFramework(String[], URL[]) line: 319 Main.basicRun(String[]) line: 272 Main.run(String[]) line: 951 Main.main(String[]) line: 935 An easy fix might be swapping the two lines in remove triggeringOperation = null; history.replaceOperation(this, (IUndoableOperation[]) children .toArray(new IUndoableOperation[children.size()])); Susan, I don't know the code well however flushing the undo histroy via a async exec seems strange. Why is the necessary ? Wouldn't it make sense to flush the histroy synchron and update the UI on the event asynchornously.
bundles/org.eclipse.core.commands/src/org/eclipse/core/commands/operations/TriggeredOperations.java
if (triggeringOperation.getContexts().length == 1) {
				remove(triggeringOperation);
				return;
			}
			triggeringOperation.removeContext(context);
			recomputeContexts();
		}
		
		ArrayList toBeRemoved = new ArrayList();
		for (int i = 0; i < children.size(); i++) {
			IUndoableOperation child = (IUndoableOperation) children.get(i);
			if (child.hasContext(context)) {
				if (child.getContexts().length == 1) {
					toBeRemoved.add(child);
				} else {
					child.removeContext(context);
					recomputeContexts();
				}
			}
		}
		for (int i = 0; i < toBeRemoved.size(); i++) {
			remove((IUndoableOperation) toBeRemoved.get(i));
		}
	}
	/*
	 * (non-Javadoc)
	 * 
	 * @see org.eclipse.core.commands.operations.IUndoableOperation#execute(org.eclipse.core.runtime.IProgressMonitor,
	 *      org.eclipse.core.runtime.IAdaptable)
	 */