IDEA-57643 do not put contents into vcs dirty scope manager under r/w lock
authorirengrig <Irina.Chernushina@jetbrains.com>
Fri, 20 Aug 2010 10:31:57 +0000 (14:31 +0400)
committerirengrig <Irina.Chernushina@jetbrains.com>
Fri, 20 Aug 2010 11:44:24 +0000 (15:44 +0400)
platform/vcs-api/src/com/intellij/openapi/vcs/changes/VcsDirtyScopeManager.java
platform/vcs-impl/src/com/intellij/openapi/vcs/changes/LockFreeRunnable.java [new file with mode: 0644]
platform/vcs-impl/src/com/intellij/openapi/vcs/changes/VcsDirtyScopeManagerImpl.java
platform/vcs-impl/src/com/intellij/openapi/vcs/changes/VcsDirtyScopeManagerProxy.java
plugins/svn4idea/testSource/org/jetbrains/idea/svn/SvnTestDirtyScopeStateTest.java

index 9c4856fc0b18ea4dbe01662397f886a785cd7ad1..7569bfbb7563f8f1d38826457fd4b56e916bc003 100644 (file)
@@ -91,10 +91,10 @@ public abstract class VcsDirtyScopeManager {
   /**
    * Requests an asynchronous file status update for all files specified and under the specified directories
    */
-  public abstract boolean filePathsDirty(@Nullable final Collection<FilePath> filesDirty, @Nullable final Collection<FilePath> dirsRecursivelyDirty);
+  public abstract void filePathsDirty(@Nullable final Collection<FilePath> filesDirty, @Nullable final Collection<FilePath> dirsRecursivelyDirty);
 
   /**
    * Requests an asynchronous file status update for all files specified and under the specified directories
    */
-  public abstract boolean filesDirty(@Nullable final Collection<VirtualFile> filesDirty, @Nullable final Collection<VirtualFile> dirsRecursivelyDirty);
+  public abstract void filesDirty(@Nullable final Collection<VirtualFile> filesDirty, @Nullable final Collection<VirtualFile> dirsRecursivelyDirty);
 }
diff --git a/platform/vcs-impl/src/com/intellij/openapi/vcs/changes/LockFreeRunnable.java b/platform/vcs-impl/src/com/intellij/openapi/vcs/changes/LockFreeRunnable.java
new file mode 100644 (file)
index 0000000..39a93cb
--- /dev/null
@@ -0,0 +1,44 @@
+/*
+ * Copyright 2000-2010 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.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.intellij.openapi.vcs.changes;
+
+import com.intellij.openapi.application.ApplicationManager;
+import com.intellij.openapi.application.ex.ApplicationEx;
+
+/**
+ * @author irengrig
+ */
+public class LockFreeRunnable implements Runnable {
+  private final Runnable myDelegate;
+
+  private LockFreeRunnable(Runnable delegate) {
+    myDelegate = delegate;
+  }
+
+  @Override
+  public void run() {
+    final ApplicationEx application = (ApplicationEx) ApplicationManager.getApplication();
+    if (application.isWriteAccessAllowed() || application.holdsReadLock()) {
+      application.executeOnPooledThread(myDelegate);
+    } else {
+      myDelegate.run();
+    }
+  }
+
+  public static Runnable wrap(final Runnable runnable) {
+    return new LockFreeRunnable(runnable);
+  }
+}
index d4035cdae94111461f64e7f71aebb676a1203f23..eef0c897cc4b45ef2a86e5c91705d3fbf416f06e 100644 (file)
@@ -143,7 +143,7 @@ public class VcsDirtyScopeManagerImpl extends VcsDirtyScopeManager implements Pr
     }
   }
 
-  public boolean filePathsDirty(@Nullable final Collection<FilePath> filesDirty, @Nullable final Collection<FilePath> dirsRecursivelyDirty) {
+  public void filePathsDirty(@Nullable final Collection<FilePath> filesDirty, @Nullable final Collection<FilePath> dirsRecursivelyDirty) {
     final ArrayList<FilePathUnderVcs> filesConverted = filesDirty == null ? null : new ArrayList<FilePathUnderVcs>(filesDirty.size());
     final ArrayList<FilePathUnderVcs> dirsConverted = dirsRecursivelyDirty == null ? null : new ArrayList<FilePathUnderVcs>(dirsRecursivelyDirty.size());
 
@@ -155,9 +155,9 @@ public class VcsDirtyScopeManagerImpl extends VcsDirtyScopeManager implements Pr
     });
     final boolean haveStuff = filesConverted != null && ! filesConverted.isEmpty()
                               || dirsConverted != null && ! dirsConverted.isEmpty();
-    if (! haveStuff) return false;
+    if (! haveStuff) return;
 
-    return takeDirt(new Consumer<DirtBuilder>() {
+    takeDirt(new Consumer<DirtBuilder>() {
       public void consume(final DirtBuilder dirt) {
         if (filesConverted != null) {
           for (FilePathUnderVcs root : filesConverted) {
@@ -173,21 +173,24 @@ public class VcsDirtyScopeManagerImpl extends VcsDirtyScopeManager implements Pr
     });
   }
 
-  private boolean takeDirt(final Consumer<DirtBuilder> filler) {
-    final Ref<Boolean> wasNotEmptyRef = new Ref<Boolean>();
-    final Runnable runnable = new Runnable() {
+  private void takeDirt(final Consumer<DirtBuilder> filler) {
+    LockFreeRunnable.wrap(new Runnable() {
+      @Override
       public void run() {
-        filler.consume(myDirtBuilder);
-        wasNotEmptyRef.set(!myDirtBuilder.isEmpty());
-      }
-    };
-    final LifeDrop lifeDrop = myLife.doIfAlive(runnable);
+        final Ref<Boolean> wasNotEmptyRef = new Ref<Boolean>();
+        final Runnable runnable = new Runnable() {
+          public void run() {
+            filler.consume(myDirtBuilder);
+            wasNotEmptyRef.set(!myDirtBuilder.isEmpty());
+          }
+        };
+        final LifeDrop lifeDrop = myLife.doIfAlive(runnable);
 
-    if (lifeDrop.isDone() && !lifeDrop.isSuspened() && Boolean.TRUE.equals(wasNotEmptyRef.get())) {
-      myChangeListManager.scheduleUpdate();
-    }
-    // no sense in checking correct here any more: vcs is searched for asynchronously
-    return !lifeDrop.isDone();
+        if (lifeDrop.isDone() && !lifeDrop.isSuspened() && Boolean.TRUE.equals(wasNotEmptyRef.get())) {
+          myChangeListManager.scheduleUpdate();
+        }
+      }
+    }).run();
   }
 
   private void convert(@Nullable final Collection<VirtualFile> from, final Collection<VcsRoot> to) {
@@ -201,7 +204,7 @@ public class VcsDirtyScopeManagerImpl extends VcsDirtyScopeManager implements Pr
     }
   }
 
-  public boolean filesDirty(@Nullable final Collection<VirtualFile> filesDirty, @Nullable final Collection<VirtualFile> dirsRecursivelyDirty) {
+  public void filesDirty(@Nullable final Collection<VirtualFile> filesDirty, @Nullable final Collection<VirtualFile> dirsRecursivelyDirty) {
     final ArrayList<VcsRoot> filesConverted = filesDirty == null ? null : new ArrayList<VcsRoot>(filesDirty.size());
     final ArrayList<VcsRoot> dirsConverted = dirsRecursivelyDirty == null ? null : new ArrayList<VcsRoot>(dirsRecursivelyDirty.size());
 
@@ -212,9 +215,9 @@ public class VcsDirtyScopeManagerImpl extends VcsDirtyScopeManager implements Pr
       }
     });
     final boolean haveStuff = filesConverted != null && ! filesConverted.isEmpty() || dirsConverted != null && ! dirsConverted.isEmpty();
-    if (! haveStuff) return false;
+    if (! haveStuff) return;
 
-    return takeDirt(new Consumer<DirtBuilder>() {
+    takeDirt(new Consumer<DirtBuilder>() {
       public void consume(final DirtBuilder dirt) {
         if (filesConverted != null) {
           for (VcsRoot root : filesConverted) {
index acbb329810f246663b81db0bbce4c782be32f941..455c87395ceee5b9fd261a2dd14d1f02712cf583 100644 (file)
@@ -84,24 +84,24 @@ class VcsDirtyScopeManagerProxy extends VcsDirtyScopeManager {
     throw new UnsupportedOperationException();
   }
 
-  public boolean filePathsDirty(@Nullable final Collection<FilePath> filesDirty, @Nullable final Collection<FilePath> dirsRecursivelyDirty) {
+  public void filePathsDirty(@Nullable final Collection<FilePath> filesDirty, @Nullable final Collection<FilePath> dirsRecursivelyDirty) {
     if (filesDirty != null) {
       myFiles.addAll(filesDirty);
     }
     if (dirsRecursivelyDirty != null) {
       myDirs.addAll(dirsRecursivelyDirty);
     }
-    return true;
+    return;
   }
 
-  public boolean filesDirty(@Nullable final Collection<VirtualFile> filesDirty, @Nullable final Collection<VirtualFile> dirsRecursivelyDirty) {
+  public void filesDirty(@Nullable final Collection<VirtualFile> filesDirty, @Nullable final Collection<VirtualFile> dirsRecursivelyDirty) {
     if (filesDirty != null) {
       myVFiles.addAll(filesDirty);
     }
     if (dirsRecursivelyDirty != null) {
       myVDirs.addAll(dirsRecursivelyDirty);
     }
-    return true;
+    return;
   }
 
   public void callRealManager(final VcsDirtyScopeManager manager) {
index 203717ee7ca51c2a45429404daa0c99e2a1dea54..4e21a763431d3caf72315e50a9f15df232cc8070 100644 (file)
@@ -1,5 +1,6 @@
 package org.jetbrains.idea.svn;
 
+import com.intellij.openapi.application.ApplicationManager;
 import com.intellij.openapi.vcs.FilePath;
 import com.intellij.openapi.vcs.FilePathImpl;
 import com.intellij.openapi.vcs.ObjectsConvertor;
@@ -66,4 +67,46 @@ public class SvnTestDirtyScopeStateTest extends SvnTestCase {
     Assert.assertTrue(! dirty3.contains(new FilePathImpl(fileC)));
     Assert.assertTrue(! dirty3.contains(new FilePathImpl(fileD)));
   }
+
+  @Test
+  public void testOkToAddScopeUnderWriteAction() throws Exception {
+    enableSilentOperation(VcsConfiguration.StandardConfirmation.ADD);
+
+    final VcsDirtyScopeManagerImpl vcsDirtyScopeManager = (VcsDirtyScopeManagerImpl) VcsDirtyScopeManager.getInstance(myProject);
+
+    final VirtualFile file = createFileInCommand("a.txt", "old content");
+    final VirtualFile fileB = createFileInCommand("b.txt", "old content");
+    final VirtualFile fileC = createFileInCommand("c.txt", "old content");
+    final VirtualFile fileD = createFileInCommand("d.txt", "old content");
+
+    final List<FilePath> list = ObjectsConvertor.vf2fp(Arrays.asList(file, fileB, fileC, fileD));
+
+    vcsDirtyScopeManager.retrieveScopes();
+    vcsDirtyScopeManager.changesProcessed();
+
+    ApplicationManager.getApplication().runWriteAction(new Runnable() {
+      @Override
+      public void run() {
+        vcsDirtyScopeManager.fileDirty(file);
+        vcsDirtyScopeManager.fileDirty(fileB);
+      }
+    });
+
+    final FilePathImpl fp = new FilePathImpl(file);
+    final FilePathImpl fpB = new FilePathImpl(fileB);
+    final long start = System.currentTimeMillis();
+    while (System.currentTimeMillis() < (start + 3000)) {
+      synchronized (this) {
+        try {
+          wait(50);
+        }
+        catch (InterruptedException e) {
+          //
+        }
+      }
+      final Collection<FilePath> dirty1 = vcsDirtyScopeManager.whatFilesDirty(list);
+      if (dirty1.contains(fp) && dirty1.contains(fpB)) return;
+    }
+    Assert.assertTrue(false);
+  }
 }