From: Duarte Meneses Date: Tue, 15 Mar 2016 13:58:03 +0000 (+0100) Subject: SONAR-7124 Concurrent access to WS cache allowed X-Git-Tag: 5.5-M11~134 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=a280429889a21f2f258041caa42264570017c31a;p=sonarqube.git SONAR-7124 Concurrent access to WS cache allowed --- diff --git a/it/it-tests/src/test/java/it/analysis/IssuesModeTest.java b/it/it-tests/src/test/java/it/analysis/IssuesModeTest.java index c2f22d516c6..8f0a8c826ca 100644 --- a/it/it-tests/src/test/java/it/analysis/IssuesModeTest.java +++ b/it/it-tests/src/test/java/it/analysis/IssuesModeTest.java @@ -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() { 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); } }); diff --git a/sonar-batch/src/main/java/org/sonar/batch/scan/MutableProjectReactorProvider.java b/sonar-batch/src/main/java/org/sonar/batch/scan/MutableProjectReactorProvider.java index c3d34a23a30..afde1e18c4d 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/scan/MutableProjectReactorProvider.java +++ b/sonar-batch/src/main/java/org/sonar/batch/scan/MutableProjectReactorProvider.java @@ -19,11 +19,8 @@ */ 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); - } - } } diff --git a/sonar-batch/src/main/java/org/sonar/batch/scan/ProjectLock.java b/sonar-batch/src/main/java/org/sonar/batch/scan/ProjectLock.java index d37987adfe6..d52f49cfd88 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/scan/ProjectLock.java +++ b/sonar-batch/src/main/java/org/sonar/batch/scan/ProjectLock.java @@ -20,24 +20,30 @@ 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()) { diff --git a/sonar-batch/src/main/java/org/sonar/batch/scan/ProjectScanContainer.java b/sonar-batch/src/main/java/org/sonar/batch/scan/ProjectScanContainer.java index f92f07acce8..ff8108e4871 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/scan/ProjectScanContainer.java +++ b/sonar-batch/src/main/java/org/sonar/batch/scan/ProjectScanContainer.java @@ -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 index 00000000000..b4857bf6afb --- /dev/null +++ b/sonar-batch/src/main/java/org/sonar/batch/scan/WorkDirectoryCleaner.java @@ -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 stream = list()) { + + Iterator 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 list() throws IOException { + return Files.newDirectoryStream(workDir, new DirectoryStream.Filter() { + @Override + public boolean accept(Path entry) throws IOException { + return !DirectoryLock.LOCK_FILE_NAME.equals(entry.getFileName().toString()); + } + }); + } +} diff --git a/sonar-batch/src/main/java/org/sonar/batch/scan/filesystem/FileIndexer.java b/sonar-batch/src/main/java/org/sonar/batch/scan/filesystem/FileIndexer.java index 0ed036192b2..9ebf91886a0 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/scan/filesystem/FileIndexer.java +++ b/sonar-batch/src/main/java/org/sonar/batch/scan/filesystem/FileIndexer.java @@ -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; diff --git a/sonar-batch/src/test/java/org/sonar/batch/scan/ProjectLockTest.java b/sonar-batch/src/test/java/org/sonar/batch/scan/ProjectLockTest.java index d2139dd1bfd..c03240b7341 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/scan/ProjectLockTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/scan/ProjectLockTest.java @@ -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 index 00000000000..8bd3d5c4e3a --- /dev/null +++ b/sonar-batch/src/test/java/org/sonar/batch/scan/WorkDirectoryCleanerTest.java @@ -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); + } + +} diff --git a/sonar-home/src/main/java/org/sonar/home/cache/DirectoryLock.java b/sonar-home/src/main/java/org/sonar/home/cache/DirectoryLock.java index 64dcb87a10a..81c1a2b5352 100644 --- a/sonar-home/src/main/java/org/sonar/home/cache/DirectoryLock.java +++ b/sonar-home/src/main/java/org/sonar/home/cache/DirectoryLock.java @@ -20,16 +20,13 @@ 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()); - } } } diff --git a/sonar-home/src/test/java/org/sonar/home/cache/DirectoryLockTest.java b/sonar-home/src/test/java/org/sonar/home/cache/DirectoryLockTest.java index b6deb72da3e..088b7b1f2f8 100644 --- a/sonar-home/src/test/java/org/sonar/home/cache/DirectoryLockTest.java +++ b/sonar-home/src/test/java/org/sonar/home/cache/DirectoryLockTest.java @@ -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)