From e9c1a15ee74d6cebac5e218b2d22d28e905c8a27 Mon Sep 17 00:00:00 2001 From: Alexey Kudravtsev Date: Tue, 23 Sep 2014 18:53:54 +0400 Subject: [PATCH] stress tests crash with PCE --- .../ex/EntryPointsManagerBase.java | 10 +-- .../reference/RefJavaManagerImpl.java | 20 ++++- .../concurrency/ApplierCompleter.java | 75 +++++++++++-------- .../intellij/concurrency/JobLauncherImpl.java | 5 ++ .../com/intellij/concurrency/JobUtilTest.java | 73 ++++++++++++------ 5 files changed, 122 insertions(+), 61 deletions(-) diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/ex/EntryPointsManagerBase.java b/java/java-analysis-impl/src/com/intellij/codeInspection/ex/EntryPointsManagerBase.java index 5f098bd87230..77c408809525 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/ex/EntryPointsManagerBase.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/ex/EntryPointsManagerBase.java @@ -1,5 +1,5 @@ /* - * Copyright 2000-2013 JetBrains s.r.o. + * Copyright 2000-2014 JetBrains s.r.o. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -24,6 +24,7 @@ import com.intellij.openapi.extensions.ExtensionPoint; import com.intellij.openapi.extensions.ExtensionPointListener; import com.intellij.openapi.extensions.Extensions; import com.intellij.openapi.extensions.PluginDescriptor; +import com.intellij.openapi.extensions.impl.ExtensionPointImpl; import com.intellij.openapi.project.Project; import com.intellij.openapi.util.*; import com.intellij.profile.codeInspection.InspectionProfileManager; @@ -82,11 +83,10 @@ public abstract class EntryPointsManagerBase extends EntryPointsManager implemen public EntryPointsManagerBase(Project project) { myProject = project; myTemporaryEntryPoints = new HashSet(); - myPersistentEntryPoints = - new LinkedHashMap(); // To keep the order between readExternal to writeExternal + myPersistentEntryPoints = new LinkedHashMap(); // To keep the order between readExternal to writeExternal Disposer.register(project, this); final ExtensionPoint point = Extensions.getRootArea().getExtensionPoint(ToolExtensionPoints.DEAD_CODE_TOOL); - point.addExtensionPointListener(new ExtensionPointListener() { + ((ExtensionPointImpl)point).addExtensionPointListener(new ExtensionPointListener() { @Override public void extensionAdded(@NotNull EntryPoint extension, @Nullable PluginDescriptor pluginDescriptor) { extensionRemoved(extension, pluginDescriptor); @@ -105,7 +105,7 @@ public abstract class EntryPointsManagerBase extends EntryPointsManager implemen }); } } - }, this); + }, false, this); } public static EntryPointsManagerBase getInstance(Project project) { diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/reference/RefJavaManagerImpl.java b/java/java-analysis-impl/src/com/intellij/codeInspection/reference/RefJavaManagerImpl.java index d9291660f9be..4d6507823e17 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/reference/RefJavaManagerImpl.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/reference/RefJavaManagerImpl.java @@ -15,7 +15,10 @@ */ package com.intellij.codeInspection.reference; -import com.intellij.codeInspection.*; +import com.intellij.codeInspection.BatchSuppressManager; +import com.intellij.codeInspection.InspectionProfileEntry; +import com.intellij.codeInspection.InspectionsBundle; +import com.intellij.codeInspection.SuppressionUtil; import com.intellij.codeInspection.deadCode.UnusedDeclarationInspection; import com.intellij.codeInspection.ex.*; import com.intellij.openapi.diagnostic.Logger; @@ -35,6 +38,9 @@ import org.jdom.Element; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import javax.swing.*; + + /** * @author anna * Date: 20-Dec-2007 @@ -360,7 +366,17 @@ public class RefJavaManagerImpl extends RefJavaManager { public EntryPointsManager getEntryPointsManager() { if (myEntryPointsManager == null) { final Project project = myRefManager.getProject(); - myEntryPointsManager = EntryPointsManager.getInstance(project); + myEntryPointsManager = new EntryPointsManagerBase(project) { + @Override + public void configureAnnotations() { + + } + + @Override + public JButton createConfigureAnnotationsBtn() { + return null; + } + }; ((EntryPointsManagerBase)myEntryPointsManager).addAllPersistentEntries(EntryPointsManagerBase.getInstance(project)); } return myEntryPointsManager; diff --git a/platform/platform-impl/src/com/intellij/concurrency/ApplierCompleter.java b/platform/platform-impl/src/com/intellij/concurrency/ApplierCompleter.java index 4db4b3c67407..5530ba1cb410 100644 --- a/platform/platform-impl/src/com/intellij/concurrency/ApplierCompleter.java +++ b/platform/platform-impl/src/com/intellij/concurrency/ApplierCompleter.java @@ -1,5 +1,5 @@ /* - * Copyright 2000-2013 JetBrains s.r.o. + * Copyright 2000-2014 JetBrains s.r.o. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,9 +16,11 @@ package com.intellij.concurrency; import com.intellij.openapi.application.ex.ApplicationManagerEx; +import com.intellij.openapi.progress.ProcessCanceledException; import com.intellij.openapi.progress.ProgressIndicator; import com.intellij.openapi.progress.ProgressManager; import com.intellij.util.Processor; +import com.intellij.util.concurrency.AtomicFieldUpdater; import jsr166e.CountedCompleter; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -50,6 +52,7 @@ public class ApplierCompleter extends CountedCompleter { private final int hi; private final ApplierCompleter next; // keeps track of right-hand-side tasks volatile Throwable throwable; + private static final AtomicFieldUpdater throwableUpdater = AtomicFieldUpdater.forFieldOfType(ApplierCompleter.class, Throwable.class); // if not null, the read action has failed and this list contains unfinished subtasks private List failedSubTasks; @@ -76,7 +79,7 @@ public class ApplierCompleter extends CountedCompleter { @Override public void compute() { - compute(new Runnable() { + wrapInReadActionAndIndicator(new Runnable() { @Override public void run() { execAndForkSubTasks(); @@ -84,7 +87,7 @@ public class ApplierCompleter extends CountedCompleter { }); } - private void compute(@NotNull final Runnable process) { + private void wrapInReadActionAndIndicator(@NotNull final Runnable process) { Runnable toRun = runInReadAction ? new Runnable() { @Override public void run() { @@ -116,10 +119,12 @@ public class ApplierCompleter extends CountedCompleter { try { for (int i = lo; i < hi; ++i) { progressIndicator.checkCanceled(); - if (!processor.process(array.get(i))) throw new ComputationAbortedException(); + if (!processor.process(array.get(i))) { + throw new ComputationAbortedException(); + } long finish = System.currentTimeMillis(); long elapsed = finish - start; - if (elapsed > 10 && hi - i >= 2 && getSurplusQueuedTaskCount() <= JobSchedulerImpl.CORES_COUNT) { + if (elapsed > 5 && hi - i >= 2 && getSurplusQueuedTaskCount() <= JobSchedulerImpl.CORES_COUNT) { int mid = i + hi >>> 1; right = new ApplierCompleter(this, runInReadAction, progressIndicator, array, processor, mid, hi, right); //children.add(right); @@ -132,7 +137,7 @@ public class ApplierCompleter extends CountedCompleter { // traverse the list looking for a task available for stealing if (right != null) { - right.tryToExecAllList(); + throwable = right.tryToExecAllList(); } } catch (Throwable e) { @@ -140,40 +145,47 @@ public class ApplierCompleter extends CountedCompleter { throwable = e; } finally { - doComplete(throwable == null ? this.throwable : throwable); + doComplete(moreImportant(throwable, this.throwable)); } return right; } + private static Throwable moreImportant(Throwable throwable1, Throwable throwable2) { + Throwable result; + if (throwable1 == null) { + result = throwable2; + } + else if (throwable2 == null) { + result = throwable1; + } + else { + // any exception wins over PCE because the latter can be induced by canceled indicator because of the former + result = throwable1 instanceof ProcessCanceledException ? throwable2 : throwable1; + } + return result; + } + private void doComplete(Throwable throwable) { ApplierCompleter a = this; ApplierCompleter child = a; while (true) { - if (throwable != null) { - a.throwable = throwable; - } + // update parent.throwable in a thread safe way + Throwable oldThrowable; + Throwable newThrowable; + do { + oldThrowable = a.throwable; + newThrowable = moreImportant(oldThrowable, throwable); + } while (oldThrowable != newThrowable && !throwableUpdater.compareAndSet(a, oldThrowable, newThrowable)); + throwable = newThrowable; if (a.getPendingCount() == 0) { - if (throwable == null) { - a.onCompletion(child); - } - else { - a.throwable = throwable; - // currently avoid using onExceptionalCompletion since it leaks exceptions via jsr166e.ForkJoinTask.exceptionTable - a.onCompletion(child); - //a.onExceptionalCompletion(throwable, child); - } + // currently avoid using onExceptionalCompletion since it leaks exceptions via jsr166e.ForkJoinTask.exceptionTable + a.onCompletion(child); + //a.onExceptionalCompletion(throwable, child); child = a; a = (ApplierCompleter)a.getCompleter(); if (a == null) { - if (throwable == null) { - child.quietlyComplete(); - } - else { - child.throwable = throwable; - // currently avoid using completeExceptionally since it leaks exceptions via jsr166e.ForkJoinTask.exceptionTable - child.quietlyComplete(); - //child.completeExceptionally(throwable); - } + // currently avoid using completeExceptionally since it leaks exceptions via jsr166e.ForkJoinTask.exceptionTable + child.quietlyComplete(); break; } } @@ -190,14 +202,17 @@ public class ApplierCompleter extends CountedCompleter { } // tries to unfork, execute and re-link subtasks - private void tryToExecAllList() { + private Throwable tryToExecAllList() { ApplierCompleter right = this; + Throwable result = throwable; while (right != null) { if (right.tryUnfork()) { right.execAndForkSubTasks(); + result = moreImportant(result, right.throwable); } right = right.next; } + return result; } boolean completeTaskWhichFailToAcquireReadAction() { @@ -208,7 +223,7 @@ public class ApplierCompleter extends CountedCompleter { // these tasks could not be executed in the other thread; do them here for (final ApplierCompleter task : failedSubTasks) { task.failedSubTasks = null; - task.compute(new Runnable() { + task.wrapInReadActionAndIndicator(new Runnable() { @Override public void run() { for (int i = task.lo; i < task.hi; ++i) { diff --git a/platform/platform-impl/src/com/intellij/concurrency/JobLauncherImpl.java b/platform/platform-impl/src/com/intellij/concurrency/JobLauncherImpl.java index 876474b9124b..3c9ae67c1980 100644 --- a/platform/platform-impl/src/com/intellij/concurrency/JobLauncherImpl.java +++ b/platform/platform-impl/src/com/intellij/concurrency/JobLauncherImpl.java @@ -90,6 +90,11 @@ public class JobLauncherImpl extends JobLauncher { catch (ApplierCompleter.ComputationAbortedException e) { return false; } + catch (ProcessCanceledException e) { + // task1.processor returns false and the task cancels the indicator + // then task2 calls checkCancel() and get here + return false; + } catch (RuntimeException e) { throw e; } diff --git a/platform/platform-tests/testSrc/com/intellij/concurrency/JobUtilTest.java b/platform/platform-tests/testSrc/com/intellij/concurrency/JobUtilTest.java index 12fadb518c9a..2961f8faa63f 100644 --- a/platform/platform-tests/testSrc/com/intellij/concurrency/JobUtilTest.java +++ b/platform/platform-tests/testSrc/com/intellij/concurrency/JobUtilTest.java @@ -17,7 +17,7 @@ package com.intellij.concurrency; import com.intellij.codeInsight.daemon.impl.DaemonProgressIndicator; import com.intellij.openapi.application.ApplicationManager; -import com.intellij.openapi.progress.ProcessCanceledException; +import com.intellij.openapi.progress.EmptyProgressIndicator; import com.intellij.openapi.progress.ProgressIndicator; import com.intellij.openapi.progress.ProgressManager; import com.intellij.openapi.progress.util.AbstractProgressIndicatorBase; @@ -75,6 +75,13 @@ public class JobUtilTest extends PlatformLangTestCase { while (System.currentTimeMillis() < end); return COUNT.incrementAndGet(); } + private static int busySleep(int ms, Runnable whileWait) { + long end = System.currentTimeMillis() + ms; + while (System.currentTimeMillis() < end) { + whileWait.run(); + } + return COUNT.incrementAndGet(); + } public void testJobUtilCorrectlySplitsUpHugeWorkAndFinishes_Performance() throws Exception { COUNT.set(0); @@ -200,6 +207,7 @@ public class JobUtilTest extends PlatformLangTestCase { if (exception.get() != null) throw exception.get(); } + private static class MyException extends RuntimeException {} public void testExceptionalCompletion() throws Throwable { final List objects = Collections.nCopies(100000000, null); COUNT.set(0); @@ -209,35 +217,31 @@ public class JobUtilTest extends PlatformLangTestCase { public boolean process(Object o) { if (COUNT.incrementAndGet() == 100000) { System.out.println("PCE"); - throw new ProcessCanceledException(); + throw new MyException(); } return true; } }); - fail("PCE must have been thrown"); + fail("exception must have been thrown"); } - catch (ProcessCanceledException e) { + catch (MyException e) { // caught OK } } public void testNotNormalCompletion() throws Throwable { final List objects = Collections.nCopies(100000000, null); COUNT.set(0); - try { - boolean success = JobLauncher.getInstance().invokeConcurrentlyUnderProgress(objects, null, true, new Processor() { - @Override - public boolean process(Object o) { - if (COUNT.incrementAndGet() == 100000) { - System.out.println("PCE"); - return false; - } - return true; + boolean success = JobLauncher.getInstance().invokeConcurrentlyUnderProgress(objects, null, true, new Processor() { + @Override + public boolean process(Object o) { + if (COUNT.incrementAndGet() == 100000) { + System.out.println("PCE"); + return false; } - }); - assertFalse(success); - } - catch (ProcessCanceledException e) { - } + return true; + } + }); + assertFalse(success); } public void testJobUtilCompletesEvenIfCannotGrabReadAction() throws Throwable { @@ -275,7 +279,7 @@ public class JobUtilTest extends PlatformLangTestCase { public boolean process(Integer integer) { if (busySleep(1) == 1000) { System.out.println("PCE"); - throw new RuntimeException("xxx"); + throw new MyException(); } return true; } @@ -284,12 +288,9 @@ public class JobUtilTest extends PlatformLangTestCase { return true; } }); + fail("exception must have been thrown"); } - catch (ProcessCanceledException e) { - // OK - } - catch (RuntimeException e) { - assertEquals("xxx", e.getMessage()); + catch (MyException ignored) { } long finish = System.currentTimeMillis(); System.out.println("Elapsed: "+(finish-start)+"ms"); @@ -328,4 +329,28 @@ public class JobUtilTest extends PlatformLangTestCase { latch.countDown(); } } + + public void testProcessorReturningFalseDoesNotCrashTheOtherThread() { + final AtomicInteger delay = new AtomicInteger(0); + final Runnable checkCanceled = new Runnable() { + @Override + public void run() { + ProgressManager.checkCanceled(); + } + }; + Processor processor = new Processor() { + @Override + public boolean process(String s) { + busySleep(delay.incrementAndGet() % 10 + 10, checkCanceled); + return delay.get() % 100 != 0; + } + }; + for (int i=0; i<100; i++) { + ProgressIndicator indicator = new EmptyProgressIndicator(); + boolean result = JobLauncher.getInstance() + .invokeConcurrentlyUnderProgress(Collections.nCopies(10000, ""), indicator, false, false, processor); + assertFalse(indicator.isCanceled()); + assertFalse(result); + } + } }