From a807e76fe19abdaaedbf407d51c62b6e7b729f9e Mon Sep 17 00:00:00 2001 From: Kirill Kalishev Date: Mon, 7 Jun 2010 13:47:26 +0400 Subject: [PATCH] treeui race conditions look fixed --- .../ide/util/treeView/AbstractTreeUi.java | 94 ++++++++++--------- .../ide/util/treeView/BaseTreeTestCase.java | 1 + .../ide/util/treeView/TreeUiTest.java | 25 +---- 3 files changed, 54 insertions(+), 66 deletions(-) diff --git a/platform/platform-api/src/com/intellij/ide/util/treeView/AbstractTreeUi.java b/platform/platform-api/src/com/intellij/ide/util/treeView/AbstractTreeUi.java index bede9f11c7d4..9629b0db3afd 100644 --- a/platform/platform-api/src/com/intellij/ide/util/treeView/AbstractTreeUi.java +++ b/platform/platform-api/src/com/intellij/ide/util/treeView/AbstractTreeUi.java @@ -18,7 +18,6 @@ package com.intellij.ide.util.treeView; import com.intellij.ide.IdeBundle; import com.intellij.openapi.application.Application; import com.intellij.openapi.application.ApplicationManager; -import com.intellij.openapi.diagnostic.Log; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.progress.*; import com.intellij.openapi.project.IndexNotReadyException; @@ -1640,8 +1639,6 @@ public class AbstractTreeUi { private ActionCallback resetToReadyNow() { assertIsDispatchThread(); - Log.print("resetToReadyNow children=" + myUpdatingChildren + " loadedInBg=" + myLoadedInBackground + " workers=" + myActiveWorkerTasks); - DefaultMutableTreeNode[] uc = myUpdatingChildren.toArray(new DefaultMutableTreeNode[myUpdatingChildren.size()]); for (DefaultMutableTreeNode each : uc) { resetIncompleteNode(each); @@ -1764,13 +1761,18 @@ public class AbstractTreeUi { private void executeYieldingRequest(Runnable runnable, TreeUpdatePass pass) { try { - myYeildingPasses.remove(pass); - runnable.run(); - } - finally { - if (!isReleased()) { - maybeYeildingFinished(); + try { + myYeildingPasses.remove(pass); + runnable.run(); } + finally { + if (!isReleased()) { + maybeYeildingFinished(); + } + } + } + catch (ProcessCanceledException e) { + resetToReady(); } } @@ -1789,6 +1791,9 @@ public class AbstractTreeUi { if (isReady()) { myRevalidatedObjects.clear(); + setCancelRequested(false); + myResettingToReadyNow.set(false); + myInitialized.setDone(); if (canInitiateNewActivity()) { @@ -2079,8 +2084,6 @@ public class AbstractTreeUi { setCancelRequested(true); - Log.print("cancelUpdate requested isReleased=" + isReleased() + " cancelProcessed=" + isCancelProcessed()); - final ActionCallback done = new ActionCallback(); invokeLaterIfNeeded(new Runnable() { @@ -2091,19 +2094,15 @@ public class AbstractTreeUi { } if (myResettingToReadyNow.get()) { - Log.print("1"); getReady(this).notify(done); } else if (isReady()) { - Log.print("2"); resetToReadyNow(); done.setDone(); } else { if (isIdle() && hasPendingWork()) { - Log.print("3"); resetToReadyNow(); done.setDone(); } else { - Log.print("4 cancelProcessed=" + isCancelProcessed()); getReady(this).notify(done); } } @@ -2147,7 +2146,14 @@ public class AbstractTreeUi { try { progressive.run(indicator); - } finally { + } catch (ProcessCanceledException e) { + resetToReadyNow().doWhenDone(new Runnable() { + public void run() { + callback.setRejected(); + } + }); + return callback; + }finally { if (isReleased()) return new ActionCallback.Rejected(); getReady(this).doWhenDone(new Runnable() { @@ -3091,7 +3097,6 @@ public class AbstractTreeUi { private void registerWorkerTask(Runnable runnable, Object id) { synchronized (myActiveWorkerTasks) { - Log.print("worker started canDoNewActivity=" + canInitiateNewActivity() + " id=" + id); myActiveWorkerTasks.add(runnable); } } @@ -3101,9 +3106,6 @@ public class AbstractTreeUi { synchronized (myActiveWorkerTasks) { wasRemoved = myActiveWorkerTasks.remove(runnable); - if (wasRemoved) { - Log.print("worker finished id=" + id); - } } if (wasRemoved && finalizeRunnable != null) { @@ -3714,32 +3716,37 @@ public class AbstractTreeUi { final boolean checkIfInStructure, final boolean canSmartExpand) { - runDone(new Runnable() { - public void run() { - if (element.length == 0) { - runDone(onDone); - return; - } - - if (myUpdaterState != null) { - myUpdaterState.clearExpansion(); - } - - - final ActionCallback done = new ActionCallback(element.length); - done.doWhenDone(new Runnable() { - public void run() { + try { + runDone(new Runnable() { + public void run() { + if (element.length == 0) { runDone(onDone); + return; } - }).doWhenRejected(new Runnable() { - public void run() { - runDone(onDone); - } - }); - expandNext(element, 0, parentsOnly, checkIfInStructure, canSmartExpand, done, 0); - } - }); + if (myUpdaterState != null) { + myUpdaterState.clearExpansion(); + } + + + final ActionCallback done = new ActionCallback(element.length); + done.doWhenDone(new Runnable() { + public void run() { + runDone(onDone); + } + }).doWhenRejected(new Runnable() { + public void run() { + runDone(onDone); + } + }); + + expandNext(element, 0, parentsOnly, checkIfInStructure, canSmartExpand, done, 0); + } + }); + } + catch (ProcessCanceledException e) { + runDone(onDone); + } } private void expandNext(final Object[] elements, @@ -4230,7 +4237,6 @@ public class AbstractTreeUi { return; } - //myRequestedExpand = null; final DefaultMutableTreeNode node = (DefaultMutableTreeNode)path.getLastPathComponent(); diff --git a/platform/platform-impl/testSrc/com/intellij/ide/util/treeView/BaseTreeTestCase.java b/platform/platform-impl/testSrc/com/intellij/ide/util/treeView/BaseTreeTestCase.java index 295b70c40954..352161748324 100644 --- a/platform/platform-impl/testSrc/com/intellij/ide/util/treeView/BaseTreeTestCase.java +++ b/platform/platform-impl/testSrc/com/intellij/ide/util/treeView/BaseTreeTestCase.java @@ -1,5 +1,6 @@ package com.intellij.ide.util.treeView; +import com.intellij.openapi.progress.ProcessCanceledException; import com.intellij.openapi.util.Condition; import com.intellij.openapi.util.Disposer; import com.intellij.openapi.util.Ref; diff --git a/platform/platform-impl/testSrc/com/intellij/ide/util/treeView/TreeUiTest.java b/platform/platform-impl/testSrc/com/intellij/ide/util/treeView/TreeUiTest.java index 31d9013de9f5..a388ca31a8da 100644 --- a/platform/platform-impl/testSrc/com/intellij/ide/util/treeView/TreeUiTest.java +++ b/platform/platform-impl/testSrc/com/intellij/ide/util/treeView/TreeUiTest.java @@ -57,7 +57,6 @@ public class TreeUiTest extends AbstractTreeBuilderTest { } public void testCancelUpdate() throws Exception { - System.out.println("TreeUiTest.testCancelUpdate --------------------"); assertInterruption(Interruption.invokeCancel); } @@ -917,8 +916,6 @@ public class TreeUiTest extends AbstractTreeBuilderTest { } private void assertInterruption(Interruption cancelled) throws Exception { - Log.print("assert interruption started "); - buildStructure(myRoot); expand(getPath("/")); @@ -937,25 +934,18 @@ public class TreeUiTest extends AbstractTreeBuilderTest { " -xunit\n" + " runner\n"); - Log.print("--------------- 1 ready=" + getBuilder().getUi().isReady()); runAndInterrupt(new MyRunnable() { public void runSafe() throws Exception { updateFrom(new NodeElement("/")); } }, "update", new NodeElement("jetbrains"), cancelled); - Log.print("--------------- 1 PASSED ready=" + getBuilder().getUi().isReady()); - - Log.print("--------------- 2 ready=" + getBuilder().getUi().isReady()); runAndInterrupt(new MyRunnable() { @Override public void runSafe() throws Exception { updateFrom(new NodeElement("/")); } }, "getChildren", new NodeElement("jetbrains"), cancelled); - Log.print("--------------- 2 PASSED ready=" + getBuilder().getUi().isReady()); - - Log.print("assert interruption finished "); } @@ -1019,8 +1009,6 @@ public class TreeUiTest extends AbstractTreeBuilderTest { private void runAndInterrupt(final Runnable action, final String interruptAction, final Object interruptElement, final Interruption interruption) throws Exception { myElementUpdate.clear(); - Log.print("runAndInterrupt action=" + interruptAction + " element=" + interruptElement + " cancelRequest=" + myCancelRequest + " interruption=" + interruption + " builder=" + getBuilder()); - final Ref thread = new Ref(); final boolean[] wasInterrupted = new boolean[] {false}; @@ -1031,22 +1019,15 @@ public class TreeUiTest extends AbstractTreeBuilderTest { } - if (thread.get() != Thread.currentThread()) { - Log.print("FFFFFFFFFFUUUUUUUUUUUUUCCCCCCCCCCCCKKKKKKKKK!!!!!!!!!!!!!!!!!!!!!!!!!!!"); - } - boolean toInterrupt = element.equals(interruptElement) && action.equals(interruptAction); - Log.print("-- onElementAction action=" + action + " element=" + element + " wasInterrupted=" + wasInterrupted[0] + " toInterrupt=" + toInterrupt + " cancelProcessed" + getBuilder().getUi().isCancelProcessed()); if (wasInterrupted[0]) { if (myCancelRequest == null) { String status = getBuilder().getUi().getStatus(); - Log.print("!!!! status=" + status); myCancelRequest = new AssertionError("Not supposed to be update after interruption request: action=" + action + " element=" + element + " interruptAction=" + interruptAction + " interruptElement=" + interruptElement); } } else { if (toInterrupt) { - Log.print("-- send interruption ready=" + getBuilder().getUi().isReady()); wasInterrupted[0] = true; switch (interruption) { case throwProcessCancelled: @@ -1933,9 +1914,9 @@ public class TreeUiTest extends AbstractTreeBuilderTest { public static TestSuite suite() { TestSuite suite = new TestSuite(); - //suite.addTestSuite(Passthrough.class); - //suite.addTestSuite(SyncUpdate.class); - //suite.addTestSuite(YieldingUpdate.class); + suite.addTestSuite(Passthrough.class); + suite.addTestSuite(SyncUpdate.class); + suite.addTestSuite(YieldingUpdate.class); suite.addTestSuite(VeryQuickBgLoadingSyncUpdate.class); suite.addTestSuite(QuickBgLoadingSyncUpdate.class); suite.addTestSuite(BgLoadingSyncUpdate.class);