From e33bacad3801e85d7a66c072b99062c1250138e6 Mon Sep 17 00:00:00 2001 From: Alexey Kudravtsev Date: Mon, 29 Sep 2014 14:10:35 +0400 Subject: [PATCH] handle nested progress indicators, moved implementation to ProgressManagerImpl --- .../progress/ProgressIndicatorProvider.java | 1 + .../openapi/progress/ProgressManager.java | 95 +------- .../components/impl/ComponentManagerImpl.java | 10 +- .../progress/impl/NonCancelableIndicator.java | 6 - .../progress/impl/ProgressManagerImpl.java | 205 +++++++++++++++--- .../{util => impl}/ProgressIndicatorTest.java | 61 +++++- 6 files changed, 245 insertions(+), 133 deletions(-) rename platform/platform-tests/testSrc/com/intellij/openapi/progress/{util => impl}/ProgressIndicatorTest.java (83%) diff --git a/platform/core-api/src/com/intellij/openapi/progress/ProgressIndicatorProvider.java b/platform/core-api/src/com/intellij/openapi/progress/ProgressIndicatorProvider.java index 3abf890e2426..0fd4e889c20d 100644 --- a/platform/core-api/src/com/intellij/openapi/progress/ProgressIndicatorProvider.java +++ b/platform/core-api/src/com/intellij/openapi/progress/ProgressIndicatorProvider.java @@ -36,6 +36,7 @@ public abstract class ProgressIndicatorProvider { } @NotNull + @Deprecated // use ProgressManager.executeNonCancelableSection() instead public abstract NonCancelableSection startNonCancelableSection(); @NotNull diff --git a/platform/core-api/src/com/intellij/openapi/progress/ProgressManager.java b/platform/core-api/src/com/intellij/openapi/progress/ProgressManager.java index 6fda1c7d3504..7cea88525fd2 100644 --- a/platform/core-api/src/com/intellij/openapi/progress/ProgressManager.java +++ b/platform/core-api/src/com/intellij/openapi/progress/ProgressManager.java @@ -19,17 +19,11 @@ import com.intellij.openapi.components.ServiceManager; import com.intellij.openapi.project.Project; import com.intellij.openapi.util.Computable; import com.intellij.openapi.util.ThrowableComputable; -import com.intellij.util.containers.ConcurrentHashSet; -import com.intellij.util.containers.SmartHashSet; -import gnu.trove.THashMap; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import javax.swing.*; -import java.util.Collection; -import java.util.Map; -import java.util.Set; public abstract class ProgressManager extends ProgressIndicatorProvider { private static class ProgressManagerHolder { @@ -50,7 +44,7 @@ public abstract class ProgressManager extends ProgressIndicatorProvider { @Override public ProgressIndicator getProgressIndicator() { - return myThreadIndicator.get(); + return null; } public static void progress(@NotNull String text) throws ProcessCanceledException { @@ -176,96 +170,21 @@ public abstract class ProgressManager extends ProgressIndicatorProvider { public abstract void runProcessWithProgressAsynchronously(@NotNull Task.Backgroundable task, @NotNull ProgressIndicator progressIndicator); - protected static final ThreadLocal myThreadIndicator = new ThreadLocal(); - - // indicator -> threads which are running under this indicator - private static final Map> threadsUnderIndicator = new THashMap>(); - // threads which are running under canceled indicator - private static final Set threadsUnderCanceledIndicator = new ConcurrentHashSet(); + public static volatile boolean ALWAYS_CHECK_CANCELED = false; + protected void indicatorCanceled(@NotNull ProgressIndicator indicator) { + } public static void canceled(@NotNull ProgressIndicator indicator) { - // mark threads running under this indicator as canceled - synchronized (threadsUnderIndicator) { - Set threads = threadsUnderIndicator.get(indicator); - if (threads != null) { - threadsUnderCanceledIndicator.addAll(threads); - } - } + getInstance().indicatorCanceled(indicator); } - public static volatile boolean ALWAYS_CHECK_CANCELED = false; public static void checkCanceled() throws ProcessCanceledException { - boolean thereIsCanceledIndicator = !threadsUnderCanceledIndicator.isEmpty(); - if (thereIsCanceledIndicator || ALWAYS_CHECK_CANCELED) { - getInstance().doCheckCanceled(); - } + getInstance().doCheckCanceled(); } - private static final Collection nonStandardIndicators = new ConcurrentHashSet(); - protected static void callCheckCancelForNonStandardIndicators() { - for (ProgressIndicator indicator : nonStandardIndicators) { - try { - indicator.checkCanceled(); - } - catch (ProcessCanceledException e) { - canceled(indicator); - } - } - } - - public void executeProcessUnderProgress(@NotNull Runnable process, @Nullable("null means reuse current progress") ProgressIndicator progress) throws ProcessCanceledException { - ProgressIndicator oldIndicator = null; - boolean set = progress != null && progress != (oldIndicator = getProgressIndicator()); - if (set) { - myThreadIndicator.set(progress); - try { - registerIndicatorAndRun(progress, Thread.currentThread(), process); - } - finally { - myThreadIndicator.set(oldIndicator); - } - } - else { - process.run(); - } - } - - private static void registerIndicatorAndRun(@NotNull ProgressIndicator progress, @NotNull Thread currentThread, @NotNull Runnable process) { - Set underIndicator; - boolean alreadyUnder; - boolean addedToPerverse; - synchronized (threadsUnderIndicator) { - underIndicator = threadsUnderIndicator.get(progress); - if (underIndicator == null) { - underIndicator = new SmartHashSet(); - threadsUnderIndicator.put(progress, underIndicator); - } - alreadyUnder = !underIndicator.add(currentThread); - addedToPerverse = !(progress instanceof StandardProgressIndicator) && nonStandardIndicators.add(progress); - } - - try { - if (progress instanceof WrappedProgressIndicator) { - registerIndicatorAndRun(((WrappedProgressIndicator)progress).getOriginalProgressIndicator(), currentThread, process); - } - else { - process.run(); - } - } - finally { - synchronized (threadsUnderIndicator) { - boolean removed = alreadyUnder || underIndicator.remove(currentThread); - if (removed && underIndicator.isEmpty()) { - threadsUnderIndicator.remove(progress); - } - threadsUnderCanceledIndicator.remove(currentThread); - if (addedToPerverse) { - nonStandardIndicators.remove(progress); - } - } - } + process.run(); } } \ No newline at end of file diff --git a/platform/core-impl/src/com/intellij/openapi/components/impl/ComponentManagerImpl.java b/platform/core-impl/src/com/intellij/openapi/components/impl/ComponentManagerImpl.java index 8bb62b8ffe6f..dfc6f6b254c3 100644 --- a/platform/core-impl/src/com/intellij/openapi/components/impl/ComponentManagerImpl.java +++ b/platform/core-impl/src/com/intellij/openapi/components/impl/ComponentManagerImpl.java @@ -108,7 +108,10 @@ public abstract class ComponentManagerImpl extends UserDataHolderBase implements Class[] componentInterfaces = myComponentsRegistry.getComponentInterfaces(); for (Class componentInterface : componentInterfaces) { - ProgressIndicatorProvider.checkCanceled(); + ProgressIndicator indicator = getProgressIndicator(); + if (indicator != null) { + indicator.checkCanceled(); + } createComponent(componentInterface); } } @@ -248,7 +251,10 @@ public abstract class ComponentManagerImpl extends UserDataHolderBase implements Class[] componentClasses = myComponentsRegistry.getComponentInterfaces(); List components = new ArrayList(componentClasses.length); for (Class interfaceClass : componentClasses) { - ProgressIndicatorProvider.checkCanceled(); + ProgressIndicator indicator = getProgressIndicator(); + if (indicator != null) { + indicator.checkCanceled(); + } Object component = getComponent(interfaceClass); if (component != null) components.add(component); } diff --git a/platform/platform-impl/src/com/intellij/openapi/progress/impl/NonCancelableIndicator.java b/platform/platform-impl/src/com/intellij/openapi/progress/impl/NonCancelableIndicator.java index 1d5b6b48bbb1..b7bee8c3c9cc 100644 --- a/platform/platform-impl/src/com/intellij/openapi/progress/impl/NonCancelableIndicator.java +++ b/platform/platform-impl/src/com/intellij/openapi/progress/impl/NonCancelableIndicator.java @@ -23,12 +23,6 @@ import com.intellij.openapi.progress.StandardProgressIndicator; import org.jetbrains.annotations.NotNull; class NonCancelableIndicator implements NonCancelableSection, StandardProgressIndicator { - protected final ProgressIndicator myOld; - - NonCancelableIndicator() { - myOld = ProgressManager.getInstance().getProgressIndicator(); - } - @Override public void done() { ProgressIndicator currentIndicator = ProgressManager.getInstance().getProgressIndicator(); diff --git a/platform/platform-impl/src/com/intellij/openapi/progress/impl/ProgressManagerImpl.java b/platform/platform-impl/src/com/intellij/openapi/progress/impl/ProgressManagerImpl.java index dd69dbc18688..3e89bd22390e 100644 --- a/platform/platform-impl/src/com/intellij/openapi/progress/impl/ProgressManagerImpl.java +++ b/platform/platform-impl/src/com/intellij/openapi/progress/impl/ProgressManagerImpl.java @@ -15,6 +15,7 @@ */ package com.intellij.openapi.progress.impl; +import com.google.common.collect.ConcurrentHashMultiset; import com.intellij.concurrency.JobScheduler; import com.intellij.openapi.Disposable; import com.intellij.openapi.application.ApplicationManager; @@ -33,16 +34,25 @@ import com.intellij.openapi.wm.WindowManager; import com.intellij.openapi.wm.ex.ProgressIndicatorEx; import com.intellij.psi.PsiLock; import com.intellij.ui.SystemNotifications; +import com.intellij.util.containers.ConcurrentHashMap; +import com.intellij.util.containers.ConcurrentHashSet; +import com.intellij.util.containers.SmartHashSet; +import gnu.trove.THashMap; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.TestOnly; import javax.swing.*; import java.awt.*; +import java.util.Collection; +import java.util.Map; +import java.util.Set; import java.util.concurrent.Future; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; public class ProgressManagerImpl extends ProgressManager implements Disposable { private static final Logger LOG = Logger.getInstance("#com.intellij.openapi.progress.impl.ProgressManagerImpl"); @@ -54,11 +64,27 @@ public class ProgressManagerImpl extends ProgressManager implements Disposable { private static final boolean DISABLED = "disabled".equals(System.getProperty("idea.ProcessCanceledException")); private final ScheduledFuture myCheckCancelledFuture; + // indicator -> threads which are running under this indicator + private static final Map> threadsUnderIndicator = new THashMap>(); + // the active indicator for the thread + private static final Map currentIndicators = new ConcurrentHashMap(); + // threads which are running under canceled indicator + static final Set threadsUnderCanceledIndicator = new ConcurrentHashSet(); + + private static final Collection nonStandardIndicators = ConcurrentHashMultiset.create(); + public ProgressManagerImpl() { myCheckCancelledFuture = JobScheduler.getScheduler().scheduleWithFixedDelay(new Runnable() { @Override public void run() { - callCheckCancelForNonStandardIndicators(); + for (ProgressIndicator indicator : nonStandardIndicators) { + try { + indicator.checkCanceled(); + } + catch (ProcessCanceledException e) { + indicatorCanceled(indicator); + } + } } }, 0, CHECK_CANCELED_DELAY_MILLIS, TimeUnit.MILLISECONDS); @@ -71,24 +97,27 @@ public class ProgressManagerImpl extends ProgressManager implements Disposable { @Override protected void doCheckCanceled() throws ProcessCanceledException { - final ProgressIndicator progress = getProgressIndicator(); - if (progress != null) { - try { - progress.checkCanceled(); - } - catch (ProcessCanceledException e) { - if (DISABLED) { - return; + boolean thereIsCanceledIndicator = !threadsUnderCanceledIndicator.isEmpty(); + if (thereIsCanceledIndicator || ALWAYS_CHECK_CANCELED) { + final ProgressIndicator progress = getProgressIndicator(); + if (progress != null) { + try { + progress.checkCanceled(); } - if (Thread.holdsLock(PsiLock.LOCK)) { - ourLockedCheckCounter++; - if (ourLockedCheckCounter > 10) { + catch (ProcessCanceledException e) { + if (DISABLED) { + return; + } + if (Thread.holdsLock(PsiLock.LOCK)) { + ourLockedCheckCounter++; + if (ourLockedCheckCounter > 10) { + ourLockedCheckCounter = 0; + } + } + else { ourLockedCheckCounter = 0; + throw e; } - } - else { - ourLockedCheckCounter = 0; - throw e; } } } @@ -97,24 +126,21 @@ public class ProgressManagerImpl extends ProgressManager implements Disposable { @NotNull @Override public final NonCancelableSection startNonCancelableSection() { - NonCancelableIndicator nonCancelor = createNonCancelableIndicator(); - myThreadIndicator.set(nonCancelor); + final ProgressIndicator myOld = ProgressManager.getInstance().getProgressIndicator(); + + NonCancelableIndicator nonCancelor = new NonCancelableIndicator() { + @Override + public void done() { + setCurrentIndicator(Thread.currentThread(), myOld); + } + }; + setCurrentIndicator(Thread.currentThread(), nonCancelor); return nonCancelor; } @Override public void executeNonCancelableSection(@NotNull Runnable runnable) { - executeProcessUnderProgress(runnable, createNonCancelableIndicator()); - } - - @NotNull - private static NonCancelableIndicator createNonCancelableIndicator() { - return new NonCancelableIndicator(){ - @Override - public void done() { - myThreadIndicator.set(myOld); - } - }; + executeProcessUnderProgress(runnable, new NonCancelableIndicator()); } @Override @@ -194,7 +220,21 @@ public class ProgressManagerImpl extends ProgressManager implements Disposable { if (progress == null || progress instanceof ProgressWindow) myCurrentUnsafeProgressCount.incrementAndGet(); try { - super.executeProcessUnderProgress(process, progress); + ProgressIndicator oldIndicator = null; + boolean set = progress != null && progress != (oldIndicator = getProgressIndicator()); + if (set) { + Thread currentThread = Thread.currentThread(); + setCurrentIndicator(currentThread, progress); + try { + registerIndicatorAndRun(progress, currentThread, oldIndicator, process); + } + finally { + setCurrentIndicator(currentThread, oldIndicator); + } + } + else { + process.run(); + } } finally { if (progress == null || progress instanceof ProgressWindow) myCurrentUnsafeProgressCount.decrementAndGet(); @@ -202,6 +242,103 @@ public class ProgressManagerImpl extends ProgressManager implements Disposable { } } + private static void registerIndicatorAndRun(@NotNull ProgressIndicator indicator, + @NotNull Thread currentThread, + ProgressIndicator oldIndicator, + @NotNull Runnable process) { + Set underIndicator; + boolean alreadyUnder; + boolean isStandard; + synchronized (threadsUnderIndicator) { + underIndicator = threadsUnderIndicator.get(indicator); + if (underIndicator == null) { + underIndicator = new SmartHashSet(); + threadsUnderIndicator.put(indicator, underIndicator); + } + alreadyUnder = !underIndicator.add(currentThread); + isStandard = indicator instanceof StandardProgressIndicator; + if (!isStandard) { + nonStandardIndicators.add(indicator); + } + + if (indicator.isCanceled()) { + threadsUnderCanceledIndicator.add(currentThread); + } + else { + threadsUnderCanceledIndicator.remove(currentThread); + } + } + + try { + if (indicator instanceof WrappedProgressIndicator) { + registerIndicatorAndRun(((WrappedProgressIndicator)indicator).getOriginalProgressIndicator(), currentThread, oldIndicator, process); + } + else { + process.run(); + } + } + finally { + synchronized (threadsUnderIndicator) { + boolean removed = alreadyUnder || underIndicator.remove(currentThread); + if (removed && underIndicator.isEmpty()) { + threadsUnderIndicator.remove(indicator); + } + if (!isStandard) { + nonStandardIndicators.remove(indicator); + } + // by this time oldIndicator may have been canceled + if (oldIndicator != null && oldIndicator.isCanceled()) { + threadsUnderCanceledIndicator.add(currentThread); + } + else { + threadsUnderCanceledIndicator.remove(currentThread); + } + } + } + } + + @TestOnly + public static void runWithAlwaysCheckingCanceled(@NotNull Runnable runnable) { + Thread fake = new Thread(); + try { + threadsUnderCanceledIndicator.add(fake); + runnable.run(); + } + finally { + threadsUnderCanceledIndicator.remove(fake); + } + } + + @Override + protected void indicatorCanceled(@NotNull ProgressIndicator indicator) { + // mark threads running under this indicator as canceled + synchronized (threadsUnderIndicator) { + Set threads = threadsUnderIndicator.get(indicator); + if (threads != null) { + for (Thread thread : threads) { + ProgressIndicator currentIndicator = currentIndicators.get(thread); + if (currentIndicator == indicator) { + threadsUnderCanceledIndicator.add(thread); + } + } + } + } + } + + private static void setCurrentIndicator(@NotNull Thread currentThread, ProgressIndicator indicator) { + if (indicator == null) { + currentIndicators.remove(currentThread); + } + else { + currentIndicators.put(currentThread, indicator); + } + } + + @Override + public ProgressIndicator getProgressIndicator() { + return currentIndicators.get(Thread.currentThread()); + } + @Override public boolean runProcessWithProgressSynchronously(@NotNull final Runnable process, @NotNull String progressTitle, @@ -215,8 +352,8 @@ public class ProgressManagerImpl extends ProgressManager implements Disposable { @NotNull @Nls String progressTitle, boolean canBeCanceled, @Nullable Project project) throws E { - final Ref result = new Ref(); - final Ref exception = new Ref(); + final AtomicReference result = new AtomicReference(); + final AtomicReference exception = new AtomicReference(); runProcessWithProgressSynchronously(new Task.Modal(project, progressTitle, canBeCanceled) { @Override @@ -231,8 +368,8 @@ public class ProgressManagerImpl extends ProgressManager implements Disposable { } }, null); - if (!exception.isNull()) { - Throwable t = exception.get(); + Throwable t = exception.get(); + if (t != null) { if (t instanceof Error) throw (Error)t; if (t instanceof RuntimeException) throw (RuntimeException)t; @SuppressWarnings("unchecked") E e = (E)t; diff --git a/platform/platform-tests/testSrc/com/intellij/openapi/progress/util/ProgressIndicatorTest.java b/platform/platform-tests/testSrc/com/intellij/openapi/progress/impl/ProgressIndicatorTest.java similarity index 83% rename from platform/platform-tests/testSrc/com/intellij/openapi/progress/util/ProgressIndicatorTest.java rename to platform/platform-tests/testSrc/com/intellij/openapi/progress/impl/ProgressIndicatorTest.java index 647b0088df34..6377455e15a7 100644 --- a/platform/platform-tests/testSrc/com/intellij/openapi/progress/util/ProgressIndicatorTest.java +++ b/platform/platform-tests/testSrc/com/intellij/openapi/progress/impl/ProgressIndicatorTest.java @@ -13,13 +13,16 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package com.intellij.openapi.progress.util; +package com.intellij.openapi.progress.impl; +import com.intellij.ide.util.DelegatingProgressIndicator; import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.application.ModalityState; import com.intellij.openapi.application.ex.ApplicationManagerEx; import com.intellij.openapi.progress.*; -import com.intellij.openapi.progress.impl.ProgressManagerImpl; +import com.intellij.openapi.progress.util.ProgressIndicatorBase; +import com.intellij.openapi.progress.util.ProgressIndicatorUtils; +import com.intellij.openapi.progress.util.ReadTask; import com.intellij.openapi.wm.ex.ProgressIndicatorEx; import com.intellij.testFramework.LightPlatformTestCase; import com.intellij.testFramework.PlatformTestUtil; @@ -113,7 +116,7 @@ public class ProgressIndicatorTest extends LightPlatformTestCase { assertTrue(averageDelay < ProgressManagerImpl.CHECK_CANCELED_DELAY_MILLIS*3); } - public void testProgressIndicatorUtils() throws Throwable { + public void testProgressIndicatorUtilsScheduleWithWriteActionPriority() throws Throwable { final AtomicBoolean insideReadAction = new AtomicBoolean(); final ProgressIndicatorBase indicator = new ProgressIndicatorBase(); ProgressIndicatorUtils.scheduleWithWriteActionPriority(indicator, new ReadTask() { @@ -308,6 +311,58 @@ public class ProgressIndicatorTest extends LightPlatformTestCase { ensureCheckCanceledCalled(indicator); } + public void testNestedIndicatorsAreCanceledRight() { + checkCanceledCalled = false; + ProgressManager.getInstance().executeProcessUnderProgress(new Runnable() { + @Override + public void run() { + assertFalse(ProgressManagerImpl.threadsUnderCanceledIndicator.contains(Thread.currentThread())); + ProgressIndicator indicator = ProgressIndicatorProvider.getGlobalProgressIndicator(); + assertTrue(indicator != null && !indicator.isCanceled()); + indicator.cancel(); + assertTrue(ProgressManagerImpl.threadsUnderCanceledIndicator.contains(Thread.currentThread())); + assertTrue(indicator.isCanceled()); + final ProgressIndicatorEx nested = new ProgressIndicatorBase(); + nested.addStateDelegate(new ProgressIndicatorStub() { + @Override + public void checkCanceled() throws ProcessCanceledException { + checkCanceledCalled = true; + } + }); + ProgressManager.getInstance().executeProcessUnderProgress(new Runnable() { + @Override + public void run() { + assertFalse(ProgressManagerImpl.threadsUnderCanceledIndicator.contains(Thread.currentThread())); + ProgressIndicator indicator2 = ProgressIndicatorProvider.getGlobalProgressIndicator(); + assertTrue(indicator2 != null && !indicator2.isCanceled()); + assertSame(indicator2, nested); + ProgressManager.checkCanceled(); + } + }, nested); + + ProgressIndicator indicator3 = ProgressIndicatorProvider.getGlobalProgressIndicator(); + assertSame(indicator, indicator3); + + assertTrue(ProgressManagerImpl.threadsUnderCanceledIndicator.contains(Thread.currentThread())); + } + }, new EmptyProgressIndicator()); + assertFalse(checkCanceledCalled); + } + + public void testWrappedIndicatorsAreSortedRight() { + EmptyProgressIndicator indicator1 = new EmptyProgressIndicator(); + DelegatingProgressIndicator indicator2 = new DelegatingProgressIndicator(indicator1); + final DelegatingProgressIndicator indicator3 = new DelegatingProgressIndicator(indicator2); + ProgressManager.getInstance().executeProcessUnderProgress(new Runnable() { + @Override + public void run() { + ProgressIndicator current = ProgressIndicatorProvider.getGlobalProgressIndicator(); + assertSame(indicator3, current); + } + }, indicator3); + assertFalse(checkCanceledCalled); + } + private static class ProgressIndicatorStub implements ProgressIndicatorEx { private volatile boolean myCanceled;