]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-7124 Concurrent access to WS cache allowed 842/head
authorDuarte Meneses <duarte.meneses@sonarsource.com>
Tue, 15 Mar 2016 13:58:03 +0000 (14:58 +0100)
committerDuarte Meneses <duarte.meneses@sonarsource.com>
Wed, 16 Mar 2016 13:16:03 +0000 (14:16 +0100)
it/it-tests/src/test/java/it/analysis/IssuesModeTest.java
sonar-batch/src/main/java/org/sonar/batch/scan/MutableProjectReactorProvider.java
sonar-batch/src/main/java/org/sonar/batch/scan/ProjectLock.java
sonar-batch/src/main/java/org/sonar/batch/scan/ProjectScanContainer.java
sonar-batch/src/main/java/org/sonar/batch/scan/WorkDirectoryCleaner.java [new file with mode: 0644]
sonar-batch/src/main/java/org/sonar/batch/scan/filesystem/FileIndexer.java
sonar-batch/src/test/java/org/sonar/batch/scan/ProjectLockTest.java
sonar-batch/src/test/java/org/sonar/batch/scan/WorkDirectoryCleanerTest.java [new file with mode: 0644]
sonar-home/src/main/java/org/sonar/home/cache/DirectoryLock.java
sonar-home/src/test/java/org/sonar/home/cache/DirectoryLockTest.java

index c2f22d516c65b1de2ae24cbf1cc453425f16fbd0..8f0a8c826caaa1b6e6e436d522909922411a8481 100644 (file)
@@ -390,20 +390,22 @@ public class IssuesModeTest {
   }
 
   @Test
-  @Ignore
-  // Disabled until SONAR-7124 is fixed
   public void concurrent_issue_mode_on_existing_project() throws Exception {
     restoreProfile("one-issue-per-line.xml");
     orchestrator.getServer().provisionProject("sample", "xoo-sample");
     orchestrator.getServer().associateProjectToQualityProfile("sample", "xoo", "one-issue-per-line");
 
-    SonarRunner runner = configureRunner("shared/xoo-sample");
+    // use same working dir, because lock file is in it
+    String workDirPath = temp.newFolder().getAbsolutePath();
+    SonarRunner runner = configureRunner("shared/xoo-sample",
+      "sonar.working.directory", workDirPath);
+
     orchestrator.executeBuild(runner);
 
-    runConcurrentIssues();
+    runConcurrentIssues(workDirPath);
   }
 
-  private void runConcurrentIssues() throws Exception {
+  private void runConcurrentIssues(final String workDirPath) throws Exception {
     // Install sonar-runner in advance to avoid concurrent unzip issues
     FileSystem fileSystem = orchestrator.getConfiguration().fileSystem();
     new SonarScannerInstaller(fileSystem).install(Version.create(SonarScanner.DEFAULT_RUNNER_VERSION), fileSystem.workspace());
@@ -415,7 +417,9 @@ public class IssuesModeTest {
       tasks.add(new Callable<BuildResult>() {
 
         public BuildResult call() throws Exception {
-          SonarScanner runner = configureRunnerIssues("shared/xoo-sample", homeDir, "sonar.it.enableWaitingSensor", "true");
+          SonarScanner runner = configureRunnerIssues("shared/xoo-sample", homeDir,
+            "sonar.it.enableWaitingSensor", "true",
+            "sonar.working.directory", workDirPath);
           return orchestrator.executeBuild(runner);
         }
       });
index c3d34a23a30d10243c2457df91f722f2d4fe2be8..afde1e18c4d2a43d5709678e225e4d9963f73ca7 100644 (file)
  */
 package org.sonar.batch.scan;
 
-import java.io.File;
-import java.io.IOException;
 import org.picocontainer.injectors.ProviderAdapter;
 import org.sonar.api.batch.bootstrap.ProjectReactor;
-import org.sonar.core.util.FileUtils;
 
 public class MutableProjectReactorProvider extends ProviderAdapter {
   private ProjectReactor reactor = null;
@@ -31,16 +28,7 @@ public class MutableProjectReactorProvider extends ProviderAdapter {
   public ProjectReactor provide(ProjectReactorBuilder builder) {
     if (reactor == null) {
       reactor = builder.execute();
-      cleanDirectory(reactor.getRoot().getWorkDir());
     }
     return reactor;
   }
-
-  private static void cleanDirectory(File dir) {
-    try {
-      FileUtils.cleanDirectory(dir);
-    } catch (IOException e) {
-      throw new IllegalStateException("Failed to recreate working directory: " + dir.getAbsolutePath(), e);
-    }
-  }
 }
index d37987adfe684ce7a44e8f731732e0712ea5dcb3..d52f49cfd884522be16404bec96505ce72eebe53 100644 (file)
 package org.sonar.batch.scan;
 
 import org.sonar.batch.bootstrap.Slf4jLogger;
-
 import org.sonar.home.cache.DirectoryLock;
 import org.picocontainer.Startable;
 import org.sonar.api.batch.bootstrap.ProjectReactor;
 
+import java.io.IOException;
 import java.nio.channels.OverlappingFileLockException;
+import java.nio.file.Files;
 import java.nio.file.Path;
 
 public class ProjectLock implements Startable {
-  public static final String LOCK_FILE_NAME = ".sonar_lock";
-
-  private DirectoryLock lock;
+  private final DirectoryLock lock;
 
   public ProjectLock(ProjectReactor projectReactor) {
-    Path directory = projectReactor.getRoot().getBaseDir().toPath();
+    Path directory = projectReactor.getRoot().getWorkDir().toPath();
+    try {
+      if (!Files.exists(directory)) {
+        Files.createDirectories(directory);
+      }
+    } catch (IOException e) {
+      throw new IllegalStateException("Failed to create work directory", e);
+    }
     this.lock = new DirectoryLock(directory.toAbsolutePath(), new Slf4jLogger());
   }
-
+  
   public void tryLock() {
     try {
       if (!lock.tryLock()) {
index f92f07acce879068890255319ef60352a461d269..ff8108e487136c2d5b43f756a0ac22db746fa3e4 100644 (file)
@@ -111,6 +111,7 @@ public class ProjectScanContainer extends ComponentContainer {
     addBatchComponents();
     lock = getComponentByType(ProjectLock.class);
     lock.tryLock();
+    getComponentByType(WorkDirectoryCleaner.class).execute();
     addBatchExtensions();
     Settings settings = getComponentByType(Settings.class);
     if (settings != null && settings.getBoolean(CoreProperties.PROFILING_LOG_PROPERTY)) {
@@ -139,6 +140,7 @@ public class ProjectScanContainer extends ComponentContainer {
       props,
       DefaultAnalysisMode.class,
       ProjectReactorBuilder.class,
+      WorkDirectoryCleaner.class,
       new MutableProjectReactorProvider(),
       new ImmutableProjectReactorProvider(),
       ProjectBuildersExecutor.class,
diff --git a/sonar-batch/src/main/java/org/sonar/batch/scan/WorkDirectoryCleaner.java b/sonar-batch/src/main/java/org/sonar/batch/scan/WorkDirectoryCleaner.java
new file mode 100644 (file)
index 0000000..b4857bf
--- /dev/null
@@ -0,0 +1,63 @@
+/*
+ * SonarQube
+ * Copyright (C) 2009-2016 SonarSource SA
+ * mailto:contact AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.batch.scan;
+
+import org.sonar.api.batch.bootstrap.ProjectReactor;
+import org.sonar.core.util.FileUtils;
+import org.sonar.home.cache.DirectoryLock;
+
+import java.io.IOException;
+import java.nio.file.DirectoryStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Iterator;
+
+public class WorkDirectoryCleaner {
+  private Path workDir;
+
+  public WorkDirectoryCleaner(ProjectReactor projectReactor) {
+    workDir = projectReactor.getRoot().getWorkDir().toPath();
+  }
+
+  public void execute() {
+    if (!Files.exists(workDir)) {
+      return;
+    }
+
+    try (DirectoryStream<Path> stream = list()) {
+
+      Iterator<Path> it = stream.iterator();
+      while (it.hasNext()) {
+        FileUtils.deleteQuietly(it.next().toFile());
+      }
+    } catch (IOException e) {
+      throw new IllegalStateException("Failed to clean working directory: " + workDir.toString(), e);
+    }
+  }
+
+  private DirectoryStream<Path> list() throws IOException {
+    return Files.newDirectoryStream(workDir, new DirectoryStream.Filter<Path>() {
+      @Override
+      public boolean accept(Path entry) throws IOException {
+        return !DirectoryLock.LOCK_FILE_NAME.equals(entry.getFileName().toString());
+      }
+    });
+  }
+}
index 0ed036192b28cd70c7c02131481ad91737c88cc6..9ebf91886a04899d8856d97abe253ea25a398754 100644 (file)
@@ -31,8 +31,8 @@ import org.sonar.api.batch.fs.internal.DefaultInputDir;
 import org.sonar.api.batch.fs.internal.DefaultInputFile;
 import org.sonar.api.scan.filesystem.PathResolver;
 import org.sonar.api.utils.MessageException;
-import org.sonar.batch.scan.ProjectLock;
 import org.sonar.batch.util.ProgressReport;
+import org.sonar.home.cache.DirectoryLock;
 
 import java.io.File;
 import java.io.IOException;
@@ -217,7 +217,7 @@ public class FileIndexer {
 
     @Override
     public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
-      if (!Files.isHidden(file) && !ProjectLock.LOCK_FILE_NAME.equals(file.getFileName().toString())) {
+      if (!Files.isHidden(file) && !DirectoryLock.LOCK_FILE_NAME.equals(file.getFileName().toString())) {
         indexFile(inputFileBuilder, fileSystem, status, file, type);
       }
       return FileVisitResult.CONTINUE;
index d2139dd1bfd59f005a05374f992a160c60f5bab7..c03240b7341eb67c6c1d959fcf8c916c763bdf69 100644 (file)
@@ -22,11 +22,11 @@ package org.sonar.batch.scan;
 import org.junit.rules.ExpectedException;
 import org.sonar.api.batch.bootstrap.ProjectDefinition;
 import org.sonar.api.batch.bootstrap.ProjectReactor;
+import org.sonar.home.cache.DirectoryLock;
 
 import java.io.File;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.nio.file.Paths;
 
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -48,25 +48,25 @@ public class ProjectLockTest {
   public void setUp() {
     lock = setUpTest(tempFolder.getRoot());
   }
-  
+
   private ProjectLock setUpTest(File file) {
     ProjectReactor projectReactor = mock(ProjectReactor.class);
     ProjectDefinition projectDefinition = mock(ProjectDefinition.class);
     when(projectReactor.getRoot()).thenReturn(projectDefinition);
-    when(projectDefinition.getBaseDir()).thenReturn(file);
+    when(projectDefinition.getWorkDir()).thenReturn(file);
 
     return new ProjectLock(projectReactor);
   }
 
   @Test
   public void tryLock() {
-    Path lockFilePath = tempFolder.getRoot().toPath().resolve(ProjectLock.LOCK_FILE_NAME);
+    Path lockFilePath = tempFolder.getRoot().toPath().resolve(DirectoryLock.LOCK_FILE_NAME);
     lock.tryLock();
     assertThat(Files.exists(lockFilePath)).isTrue();
     assertThat(Files.isRegularFile(lockFilePath)).isTrue();
 
     lock.stop();
-    assertThat(Files.exists(lockFilePath)).isFalse();
+    assertThat(Files.exists(lockFilePath)).isTrue();
   }
 
   @Test
@@ -76,7 +76,7 @@ public class ProjectLockTest {
     lock.tryLock();
     lock.tryLock();
   }
-  
+
   @Test
   /**
    * If there is an error starting up the scan, we'll still try to unlock even if the lock
@@ -94,18 +94,9 @@ public class ProjectLockTest {
     lock.tryLock();
     lock.stop();
   }
-  
-  @Test
-  public void errorLock() {
-    lock = setUpTest(Paths.get("path", "that", "wont", "exist", "ever").toFile());
-    exception.expect(IllegalStateException.class);
-    exception.expectMessage("Failed to create lock in");
-    lock.tryLock();
-  }
-  
+
   @Test
-  public void errorDeleteLock() {
-    lock = setUpTest(Paths.get("path", "that", "wont", "exist", "ever").toFile());
+  public void unLockWithNoLock() {
     lock.stop();
   }
 
diff --git a/sonar-batch/src/test/java/org/sonar/batch/scan/WorkDirectoryCleanerTest.java b/sonar-batch/src/test/java/org/sonar/batch/scan/WorkDirectoryCleanerTest.java
new file mode 100644 (file)
index 0000000..8bd3d5c
--- /dev/null
@@ -0,0 +1,79 @@
+/*
+ * SonarQube
+ * Copyright (C) 2009-2016 SonarSource SA
+ * mailto:contact AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.batch.scan;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.sonar.api.batch.bootstrap.ProjectDefinition;
+import org.sonar.api.batch.bootstrap.ProjectReactor;
+import org.sonar.home.cache.DirectoryLock;
+
+import java.io.File;
+import java.io.IOException;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+public class WorkDirectoryCleanerTest {
+  private WorkDirectoryCleaner cleaner;
+  @Rule
+  public TemporaryFolder temp = new TemporaryFolder();
+
+  @Before
+  public void setUp() throws IOException {
+    // create files to clean
+    temp.newFile();
+    File newFolder = temp.newFolder();
+    File fileInFolder = new File(newFolder, "test");
+    fileInFolder.createNewFile();
+
+    File lock = new File(temp.getRoot(), DirectoryLock.LOCK_FILE_NAME);
+    lock.createNewFile();
+
+    // mock project
+    ProjectReactor projectReactor = mock(ProjectReactor.class);
+    ProjectDefinition projectDefinition = mock(ProjectDefinition.class);
+    when(projectReactor.getRoot()).thenReturn(projectDefinition);
+    when(projectDefinition.getWorkDir()).thenReturn(temp.getRoot());
+
+    assertThat(temp.getRoot().list().length).isGreaterThan(1);
+    cleaner = new WorkDirectoryCleaner(projectReactor);
+  }
+  
+  @Test
+  public void testNonExisting() {
+    temp.delete();
+    cleaner.execute();
+  }
+  
+  @Test
+  public void testClean() {
+    File lock = new File(temp.getRoot(), DirectoryLock.LOCK_FILE_NAME);
+    cleaner.execute();
+
+    assertThat(temp.getRoot()).exists();
+    assertThat(lock).exists();
+    assertThat(temp.getRoot().list()).containsOnly(DirectoryLock.LOCK_FILE_NAME);
+  }
+
+}
index 64dcb87a10a6b026fea8774ae9f7703e20f24aed..81c1a2b5352075ab7c122ae1e5c4757c2b9e38fb 100644 (file)
 package org.sonar.home.cache;
 
 import java.io.IOException;
-import java.io.PrintWriter;
 import java.io.RandomAccessFile;
-import java.io.StringWriter;
 import java.nio.channels.FileChannel;
 import java.nio.channels.FileLock;
-import java.nio.file.Files;
 import java.nio.file.Path;
 
 public class DirectoryLock {
-  static final String LOCK_FILE_NAME = ".sonar_lock";
+  public static final String LOCK_FILE_NAME = ".sonar_lock";
   private final Path lockFilePath;
   private final Logger logger;
 
@@ -93,14 +90,5 @@ public class DirectoryLock {
         logger.error("Error closing file", e);
       }
     }
-
-    try {
-      Files.delete(lockFilePath);
-    } catch (IOException e) {
-      // ignore, as an error happens if another process just started to acquire the same lock
-      StringWriter errors = new StringWriter();
-      e.printStackTrace(new PrintWriter(errors));
-      logger.debug("Couldn't delete lock file: " + lockFilePath.toString() + " " + errors.toString());
-    }
   }
 }
index b6deb72da3e922139cc1b31b5c66f624eec0c9ca..088b7b1f2f864d2b2555a46d736127be9449b1e3 100644 (file)
@@ -50,7 +50,6 @@ public class DirectoryLockTest {
     lock.lock();
     assertThat(temp.getRoot().toPath().resolve(".sonar_lock")).exists();
     lock.unlock();
-    assertThat(temp.getRoot().list()).isEmpty();
   }
 
   @Test
@@ -59,7 +58,6 @@ public class DirectoryLockTest {
     lock.tryLock();
     assertThat(temp.getRoot().toPath().resolve(".sonar_lock")).exists();
     lock.unlock();
-    assertThat(temp.getRoot().list()).isEmpty();
   }
 
   @Test(expected = OverlappingFileLockException.class)