SONARPY-3140 Rework synchronization with concurrency package tools (#351)

GitOrigin-RevId: 7d721aacef118af22c4e3f2b44504aed198231c1

Author: maksim-grebeniuk-sonarsource

python-commons/src/main/java/org/sonar/plugins/python/IssuesRepository.java

@@ -20,6 +20,7 @@
 import java.util.ArrayDeque;
 import java.util.List;
 import java.util.Optional;
+import java.util.concurrent.locks.Lock;
 import javax.annotation.CheckForNull;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -41,14 +42,14 @@ public class IssuesRepository {
   private final PythonChecks checks;
   private final PythonIndexer indexer;
   private final boolean isInSonarLint;
-  private final Object monitor;
+  private final Lock lock;
 
-  public IssuesRepository(SensorContext context, PythonChecks checks, PythonIndexer indexer, boolean isInSonarLint, Object monitor) {
+  public IssuesRepository(SensorContext context, PythonChecks checks, PythonIndexer indexer, boolean isInSonarLint, Lock lock) {
     this.context = context;
     this.checks = checks;
     this.indexer = indexer;
     this.isInSonarLint = isInSonarLint;
-    this.monitor = monitor;
+    this.lock = lock;
   }
 
   public void save(PythonInputFile inputFile, List<PythonCheck.PreciseIssue> issues) {
@@ -94,8 +95,11 @@ private void save(PythonInputFile inputFile, PythonCheck.PreciseIssue preciseIss
   }
 
   private void save(NewIssue newIssue) {
-    synchronized (monitor) {
+    try {
+      lock.lock();
       newIssue.save();
+    } finally {
+      lock.unlock();
     }
   }
 

python-commons/src/main/java/org/sonar/plugins/python/MeasuresRepository.java

@@ -17,6 +17,7 @@
 package org.sonar.plugins.python;
 
 import java.util.Set;
+import java.util.concurrent.locks.Lock;
 import org.sonar.api.batch.sensor.SensorContext;
 import org.sonar.api.batch.sensor.measure.NewMeasure;
 import org.sonar.api.issue.NoSonarFilter;
@@ -33,17 +34,26 @@ public class MeasuresRepository {
   private final NoSonarFilter noSonarFilter;
   private final FileLinesContextFactory fileLinesContextFactory;
   private final boolean isInSonarLint;
-  private final Object monitor;
+  private final Lock lock;
 
-  public MeasuresRepository(SensorContext context, NoSonarFilter noSonarFilter, FileLinesContextFactory fileLinesContextFactory, boolean isInSonarLint, Object monitor) {
+  public MeasuresRepository(SensorContext context, NoSonarFilter noSonarFilter, FileLinesContextFactory fileLinesContextFactory, boolean isInSonarLint, Lock lock) {
     this.context = context;
     this.noSonarFilter = noSonarFilter;
     this.fileLinesContextFactory = fileLinesContextFactory;
     this.isInSonarLint = isInSonarLint;
-    this.monitor = monitor;
+    this.lock = lock;
   }
 
-  public synchronized void save(PythonInputFile inputFile, PythonVisitorContext visitorContext) {
+  public void save(PythonInputFile inputFile, PythonVisitorContext visitorContext) {
+    try {
+      lock.lock();
+      saveInternal(inputFile, visitorContext);
+    } finally {
+      lock.unlock();
+    }
+  }
+
+  private void saveInternal(PythonInputFile inputFile, PythonVisitorContext visitorContext) {
     FileMetrics fileMetrics = new FileMetrics(visitorContext, isNotebook(inputFile));
     FileLinesVisitor fileLinesVisitor = fileMetrics.fileLinesVisitor();
 
@@ -73,8 +83,11 @@ public synchronized void save(PythonInputFile inputFile, PythonVisitorContext vi
   }
 
   private void processNoSonarInFile(PythonInputFile inputFile, FileLinesVisitor fileLinesVisitor) {
-    synchronized (monitor) {
+    try {
+      lock.lock();
       noSonarFilter.noSonarInFile(inputFile.wrappedFile(), fileLinesVisitor.getLinesWithNoSonar());
+    } finally {
+      lock.unlock();
     }
   }
 
@@ -91,14 +104,20 @@ private void saveMetricOnFile(PythonInputFile inputFile, Metric<Integer> metric,
   }
 
   private void save(FileLinesContext fileLinesContext) {
-    synchronized (monitor) {
+    try {
+      lock.lock();
       fileLinesContext.save();
+    } finally {
+      lock.unlock();
     }
   }
 
   private void save(NewMeasure<Integer> measure) {
-    synchronized (monitor) {
+    try {
+      lock.lock();
       measure.save();
+    } finally {
+      lock.unlock();
     }
   }
 }

python-commons/src/main/java/org/sonar/plugins/python/NewSymbolsCollector.java

@@ -19,6 +19,7 @@
 import java.util.ArrayList;
 import java.util.Comparator;
 import java.util.List;
+import java.util.concurrent.locks.Lock;
 import org.sonar.api.batch.sensor.symbol.NewSymbol;
 import org.sonar.api.batch.sensor.symbol.NewSymbolTable;
 import org.sonar.plugins.python.api.symbols.Symbol;
@@ -33,10 +34,10 @@
 import org.sonar.plugins.python.api.tree.Tree;
 
 public class NewSymbolsCollector {
-  private final Object monitor;
+  private final Lock lock;
 
-  public NewSymbolsCollector(Object monitor) {
-    this.monitor = monitor;
+  public NewSymbolsCollector(Lock lock) {
+    this.lock = lock;
   }
 
   public void collect(NewSymbolTable newSymbolTable, FileInput fileInput) {
@@ -46,8 +47,11 @@ public void collect(NewSymbolTable newSymbolTable, FileInput fileInput) {
   }
 
   private void save(NewSymbolTable newSymbolTable) {
-    synchronized (monitor) {
+    try {
+      lock.lock();
       newSymbolTable.save();
+    } finally {
+      lock.unlock();
     }
   }
 

python-commons/src/main/java/org/sonar/plugins/python/PythonHighlighter.java

@@ -19,6 +19,8 @@
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.Set;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
 import javax.annotation.Nullable;
 import org.sonar.api.batch.sensor.SensorContext;
 import org.sonar.api.batch.sensor.highlighting.NewHighlighting;
@@ -85,14 +87,14 @@
  */
 public class PythonHighlighter {
 
-  private final Object monitor;
+  private final Lock lock;
 
-  public PythonHighlighter(Object monitor) {
-    this.monitor = monitor;
+  public PythonHighlighter(Lock lock) {
+    this.lock = lock;
   }
 
   public PythonHighlighter() {
-    this(new Object());
+    this(new ReentrantLock());
   }
 
   public void highlight(SensorContext sensorContext, PythonVisitorContext visitorContext, PythonInputFile inputFile) {
@@ -102,8 +104,11 @@ public void highlight(SensorContext sensorContext, PythonVisitorContext visitorC
   }
 
   private void save(NewHighlighting newHighlighting) {
-    synchronized (monitor) {
+    try {
+      lock.lock();
       newHighlighting.save();
+    } finally {
+      lock.unlock();
     }
   }
 

python-commons/src/main/java/org/sonar/plugins/python/PythonScanner.java

@@ -24,11 +24,14 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
 import java.util.function.Supplier;
 import java.util.regex.Pattern;
 import org.slf4j.Logger;
@@ -44,6 +47,7 @@
 import org.sonar.plugins.python.api.PythonInputFileContext;
 import org.sonar.plugins.python.api.PythonSubscriptionCheck;
 import org.sonar.plugins.python.api.PythonVisitorContext;
+import org.sonar.plugins.python.api.internal.EndOfAnalysis;
 import org.sonar.plugins.python.api.tree.FileInput;
 import org.sonar.plugins.python.cpd.PythonCpdAnalyzer;
 import org.sonar.plugins.python.indexer.PythonIndexer;
@@ -68,18 +72,18 @@ public class PythonScanner extends Scanner {
   private final AtomicInteger recognitionErrorCount;
   private final AtomicBoolean foundDatabricks;
   private final PythonFileConsumer architectureCallback;
-  private final Map<String, Object> repositoryLocks;
+  private final Map<String, Lock> repositoryLocks;
   private final NewSymbolsCollector newSymbolsCollector;
   private final PythonHighlighter pythonHighlighter;
   private final IssuesRepository issuesRepository;
   private final MeasuresRepository measuresRepository;
+  private final Lock lock;
 
   public PythonScanner(
     SensorContext context, PythonChecks checks, FileLinesContextFactory fileLinesContextFactory, NoSonarFilter noSonarFilter,
     Supplier<PythonParser> parserSupplier, PythonIndexer indexer, PythonFileConsumer architectureCallback) {
     super(context);
     this.checks = checks;
-    this.cpdAnalyzer = new PythonCpdAnalyzer(context, this);
     this.parserSupplier = parserSupplier;
     this.indexer = indexer;
     this.indexer.buildOnce(context);
@@ -88,10 +92,12 @@ public PythonScanner(
     this.recognitionErrorCount = new AtomicInteger(0);
     this.foundDatabricks = new AtomicBoolean(false);
     this.repositoryLocks = new ConcurrentHashMap<>();
-    this.newSymbolsCollector = new NewSymbolsCollector(this);
-    this.pythonHighlighter = new PythonHighlighter(this);
-    this.issuesRepository = new IssuesRepository(context, checks, indexer, isInSonarLint(context), this);
-    this.measuresRepository = new MeasuresRepository(context, noSonarFilter, fileLinesContextFactory, isInSonarLint(context), this);
+    this.lock = new ReentrantLock();
+    this.cpdAnalyzer = new PythonCpdAnalyzer(context, lock);
+    this.newSymbolsCollector = new NewSymbolsCollector(lock);
+    this.pythonHighlighter = new PythonHighlighter(lock);
+    this.issuesRepository = new IssuesRepository(context, checks, indexer, isInSonarLint(context), lock);
+    this.measuresRepository = new MeasuresRepository(context, noSonarFilter, fileLinesContextFactory, isInSonarLint(context), lock);
   }
 
   @Override
@@ -113,9 +119,8 @@ protected void scanFile(PythonInputFile inputFile) throws IOException {
     executeChecks(visitorContext, checks.sonarPythonChecks(), fileType, inputFile);
     executeOtherChecks(inputFile, visitorContext, fileType);
 
-    synchronized (repositoryLocks.computeIfAbsent(ARCHITECTURE_CALLBACK_LOCK_KEY, k -> new Object())) {
-      architectureCallback.scanFile(visitorContext);
-    }
+
+    runLockedByRepository(ARCHITECTURE_CALLBACK_LOCK_KEY, () -> architectureCallback.scanFile(visitorContext));
 
     issuesRepository.save(inputFile, visitorContext.getIssues());
 
@@ -187,13 +192,8 @@ private void executeChecks(PythonVisitorContext visitorContext, Collection<Pytho
   }
 
   private void executeOtherChecks(PythonInputFile inputFile, PythonVisitorContext visitorContext, InputFile.Type fileType) {
-    checks.noSonarPythonChecks().forEach((repositoryKey, repositoryChecks) -> {
-        var lock = repositoryLocks.computeIfAbsent(repositoryKey, k -> new Object());
-        synchronized (lock) {
-          executeChecks(visitorContext, repositoryChecks, fileType, inputFile);
-        }
-      }
-    );
+    checks.noSonarPythonChecks()
+      .forEach((repositoryKey, repositoryChecks) -> runLockedByRepository(repositoryKey, () -> executeChecks(visitorContext, repositoryChecks, fileType, inputFile)));
   }
 
   private void searchForDataBricks(PythonVisitorContext visitorContext) {
@@ -229,17 +229,15 @@ public boolean scanFileWithoutParsing(PythonInputFile inputFile) {
 
     result = scanFileWithoutParsingSonarPython(inputFile, fileType, inputFileContext, result);
 
+    var atomicResult = new AtomicBoolean(result);
     var otherChecks = checks.noSonarPythonChecks();
-    for (var entry : otherChecks.entrySet()) {
-      var repositoryKey = entry.getKey();
-      var repositoryChecks = entry.getValue();
-      var lock = repositoryLocks.computeIfAbsent(repositoryKey, k -> new Object());
-      synchronized (lock) {
-        for (var check : repositoryChecks) {
-          result = scanFileWithoutParsingNotSonarPython(inputFile, check, fileType, result, inputFileContext);
-        }
+    otherChecks.forEach((repositoryKey, repositoryChecks) -> runLockedByRepository(repositoryKey, () -> {
+      for (var check : repositoryChecks) {
+        var scanResult = scanFileWithoutParsingNotSonarPython(inputFile, check, fileType, atomicResult.get(), inputFileContext);
+        atomicResult.set(scanResult);
       }
-    }
+    }));
+    result = atomicResult.get();
 
     result &= architectureCallback.scanWithoutParsing(inputFileContext);
     if (!result) {
@@ -293,14 +291,11 @@ private boolean scanFileWithoutParsingSonarPython(PythonInputFile inputFile, Inp
   public void endOfAnalysis() {
     indexer.postAnalysis(context);
     checks.sonarPythonEndOfAnalyses().forEach(c -> c.endOfAnalysis(indexer.cacheContext()));
-    checks.noSonarPythonEndOfAnalyses().forEach(
-      (repositoryKey, endOfAnalyses) -> {
-        var lock = repositoryLocks.computeIfAbsent(repositoryKey, k -> new Object());
-        synchronized (lock) {
-          endOfAnalyses.forEach(c -> c.endOfAnalysis(indexer.cacheContext()));
-        }
-      }
-    );
+    checks.noSonarPythonEndOfAnalyses().forEach(this::endOfAnalysisForRepository);
+  }
+
+  private void endOfAnalysisForRepository(String repositoryKey, List<EndOfAnalysis> endOfAnalyses) {
+    runLockedByRepository(repositoryKey, () -> endOfAnalyses.forEach(c -> c.endOfAnalysis(indexer.cacheContext())));
   }
 
   boolean isCheckNotApplicable(PythonCheck pythonCheck, InputFile.Type fileType) {
@@ -355,4 +350,14 @@ protected int getNumberOfThreads(SensorContext context) {
     return context.config().getInt(THREADS_PROPERTY_NAME)
       .orElse(1);
   }
+
+  private void runLockedByRepository(String repositoryKey, Runnable runnable) {
+    var repositoryLock = repositoryLocks.computeIfAbsent(repositoryKey, k -> new ReentrantLock());
+    try {
+      repositoryLock.lock();
+      runnable.run();
+    } finally {
+      repositoryLock.unlock();
+    }
+  }
 }

python-commons/src/main/java/org/sonar/plugins/python/cpd/PythonCpdAnalyzer.java

@@ -21,6 +21,7 @@
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.concurrent.locks.Lock;
 import org.sonar.api.batch.fs.InputFile;
 import org.sonar.api.batch.sensor.SensorContext;
 import org.sonar.api.batch.sensor.cpd.NewCpdTokens;
@@ -43,11 +44,11 @@ public class PythonCpdAnalyzer {
   private static final Logger LOG = LoggerFactory.getLogger(PythonCpdAnalyzer.class);
 
   private final SensorContext context;
-  private final Object monitor;
+  private final Lock lock;
 
-  public PythonCpdAnalyzer(SensorContext context, Object monitor) {
+  public PythonCpdAnalyzer(SensorContext context, Lock lock) {
     this.context = context;
-    this.monitor = monitor;
+    this.lock = lock;
   }
 
   public void pushCpdTokens(InputFile inputFile, PythonVisitorContext visitorContext) {
@@ -116,22 +117,31 @@ private void saveTokensToCache(PythonVisitorContext visitorContext, List<Token>
   }
 
   private void save(NewCpdTokens cpdTokens) {
-    synchronized (monitor) {
+    try {
+      lock.lock();
       cpdTokens.save();
+    } finally {
+      lock.unlock();
     }
   }
 
   private void copyFromPrevious(CacheContext cacheContext, String dataKey, String tableKey) {
-    synchronized (monitor) {
+    try {
+      lock.lock();
       cacheContext.getWriteCache().copyFromPrevious(dataKey);
       cacheContext.getWriteCache().copyFromPrevious(tableKey);
+    } finally {
+      lock.unlock();
     }
   }
 
   private void writeToCache(CacheContext cacheContext, String fileKey, CpdSerializer.SerializationResult result) {
-    synchronized (monitor) {
+    try {
+      lock.lock();
       cacheContext.getWriteCache().write(stringTableCacheKey(fileKey), result.stringTable);
       cacheContext.getWriteCache().write(dataCacheKey(fileKey), result.data);
+    } finally {
+      lock.unlock();
     }
   }