stress tests crash with PCE

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<RefElement>();
-    myPersistentEntryPoints =
-        new LinkedHashMap<String, SmartRefElementPointer>(); // To keep the order between readExternal to writeExternal
+    myPersistentEntryPoints = new LinkedHashMap<String, SmartRefElementPointer>(); // To keep the order between readExternal to writeExternal
     Disposer.register(project, this);
     final ExtensionPoint<EntryPoint> point = Extensions.getRootArea().getExtensionPoint(ToolExtensionPoints.DEAD_CODE_TOOL);
-    point.addExtensionPointListener(new ExtensionPointListener<EntryPoint>() {
+    ((ExtensionPointImpl)point).addExtensionPointListener(new ExtensionPointListener<EntryPoint>() {
       @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) {

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;

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<Void> {
   private final int hi;
   private final ApplierCompleter next; // keeps track of right-hand-side tasks
   volatile Throwable throwable;
+  private static final AtomicFieldUpdater<ApplierCompleter, Throwable> throwableUpdater = AtomicFieldUpdater.forFieldOfType(ApplierCompleter.class, Throwable.class);
 
   // if not null, the read action has failed and this list contains unfinished subtasks
   private List<ApplierCompleter> failedSubTasks;
@@ -76,7 +79,7 @@ public class ApplierCompleter extends CountedCompleter<Void> {
 
   @Override
   public void compute() {
-    compute(new Runnable() {
+    wrapInReadActionAndIndicator(new Runnable() {
       @Override
       public void run() {
         execAndForkSubTasks();
@@ -84,7 +87,7 @@ public class ApplierCompleter extends CountedCompleter<Void> {
     });
   }
 
-  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<Void> {
     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<Void> {
 
       // 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<Void> {
       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<Void> {
   }
 
   // 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<Void> {
     // 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) {

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;
     }

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<Object> 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<Object> objects = Collections.nCopies(100000000, null);
     COUNT.set(0);
-    try {
-      boolean success = JobLauncher.getInstance().invokeConcurrentlyUnderProgress(objects, null, true, new Processor<Object>() {
-        @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<Object>() {
+      @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<String> processor = new Processor<String>() {
+      @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);
+    }
+  }
 }