Skip to content

Commit

Permalink
SLCORE-1156 Make sure exceptions are logged from executor services
Browse files Browse the repository at this point in the history
  • Loading branch information
damien-urruty-sonarsource committed Feb 7, 2025
1 parent 9cdaa08 commit d69ade5
Show file tree
Hide file tree
Showing 18 changed files with 175 additions and 82 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
/*
* SonarLint Core - Commons
* Copyright (C) 2016-2025 SonarSource SA
* mailto:info 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.sonarsource.sonarlint.core.commons.util;

import java.util.concurrent.CancellationException;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.FutureTask;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledThreadPoolExecutor;
import java.util.concurrent.SynchronousQueue;
import java.util.concurrent.ThreadFactory;
import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
import javax.annotation.CheckForNull;
import javax.annotation.Nullable;
import org.sonarsource.sonarlint.core.commons.log.SonarLintLogger;

/**
* This class should always be preferred to {@link java.util.concurrent.Executors}, except for a few cases regarding RPC read/write threads.
*/
public class FailSafeExecutors {
private static final SonarLintLogger LOG = SonarLintLogger.get();

private FailSafeExecutors() {
// utility class
}

public static ExecutorService newSingleThreadExecutor(String threadName) {
return new ThreadPoolExecutor(1, 1, 0L, TimeUnit.MILLISECONDS, new LinkedBlockingQueue<>(), r -> new Thread(r, threadName)) {
@Override
protected void afterExecute(Runnable task, @Nullable Throwable throwable) {
var extractedThrowable = extractThrowable(task, throwable);
if (extractedThrowable != null) {
LOG.error("An error occurred while executing a task in " + threadName, extractedThrowable);
}
super.afterExecute(task, throwable);
}
};
}

public static ScheduledExecutorService newSingleThreadScheduledExecutor(String threadName) {
return new ScheduledThreadPoolExecutor(1, r -> new Thread(r, threadName)) {
@Override
protected void afterExecute(Runnable task, @Nullable Throwable throwable) {
var extractedThrowable = extractThrowable(task, throwable);
if (extractedThrowable != null) {
LOG.error("An error occurred while executing a scheduled task in " + threadName, extractedThrowable);
}
super.afterExecute(task, throwable);
}
};
}

public static ExecutorService newCachedThreadPool(ThreadFactory threadFactory) {
return new ThreadPoolExecutor(0, Integer.MAX_VALUE, 60L, TimeUnit.SECONDS, new SynchronousQueue<>(), threadFactory) {
@Override
protected void afterExecute(Runnable task, @Nullable Throwable throwable) {
var extractedThrowable = extractThrowable(task, throwable);
if (extractedThrowable != null) {
LOG.error("An error occurred while executing a task in " + Thread.currentThread(), extractedThrowable);
}
super.afterExecute(task, throwable);
}
};
}

@CheckForNull
private static Throwable extractThrowable(Runnable task, @Nullable Throwable throwable) {
if (throwable != null) {
return throwable;
}
if (task instanceof FutureTask<?> futureTask) {
try {
if (futureTask.isDone() && !futureTask.isCancelled()) {
futureTask.get();
}
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
} catch (ExecutionException e) {
return e.getCause();
} catch (CancellationException e) {
// nothing to do
}
}
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,12 @@
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import org.sonarsource.sonarlint.core.commons.log.SonarLintLogger;
import org.sonarsource.sonarlint.core.commons.progress.ExecutorServiceShutdownWatchable;
import org.sonarsource.sonarlint.core.commons.progress.SonarLintCancelMonitor;
import org.sonarsource.sonarlint.core.commons.util.FailSafeExecutors;
import org.sonarsource.sonarlint.core.event.BindingConfigChangedEvent;
import org.sonarsource.sonarlint.core.event.ConnectionConfigurationAddedEvent;
import org.sonarsource.sonarlint.core.repository.config.BindingConfiguration;
Expand Down Expand Up @@ -73,8 +72,7 @@ public BindingSuggestionProvider(ConfigurationRepository configRepository, Conne
this.client = client;
this.bindingClueProvider = bindingClueProvider;
this.sonarProjectsCache = sonarProjectsCache;
this.executorService = new ExecutorServiceShutdownWatchable<>(new ThreadPoolExecutor(0, 1, 10L, TimeUnit.SECONDS,
new LinkedBlockingQueue<>(), r -> new Thread(r, "Binding Suggestion Provider")));
this.executorService = new ExecutorServiceShutdownWatchable<>(FailSafeExecutors.newSingleThreadExecutor("Binding Suggestion Provider"));
}

@EventListener
Expand Down Expand Up @@ -116,7 +114,7 @@ public Map<String, List<BindingSuggestionDto>> getBindingSuggestions(String conf
private void queueBindingSuggestionComputation(Set<String> configScopeIds, Set<String> candidateConnectionIds) {
var cancelMonitor = new SonarLintCancelMonitor();
cancelMonitor.watchForShutdown(executorService);
executorService.submit(() -> {
executorService.execute(() -> {
if (enabled.get()) {
computeAndNotifyBindingSuggestions(configScopeIds, candidateConnectionIds, cancelMonitor);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,11 @@
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import org.sonarsource.sonarlint.core.commons.log.SonarLintLogger;
import org.sonarsource.sonarlint.core.commons.progress.ExecutorServiceShutdownWatchable;
import org.sonarsource.sonarlint.core.commons.progress.SonarLintCancelMonitor;
import org.sonarsource.sonarlint.core.commons.util.FailSafeExecutors;
import org.sonarsource.sonarlint.core.event.ConfigurationScopesAddedEvent;
import org.sonarsource.sonarlint.core.fs.ClientFile;
import org.sonarsource.sonarlint.core.fs.ClientFileSystemService;
Expand Down Expand Up @@ -70,8 +68,7 @@ public ConnectionSuggestionProvider(ConfigurationRepository configRepository, Co
this.connectionRepository = connectionRepository;
this.client = client;
this.bindingClueProvider = bindingClueProvider;
this.executorService = new ExecutorServiceShutdownWatchable<>(new ThreadPoolExecutor(0, 1, 10L, TimeUnit.SECONDS,
new LinkedBlockingQueue<>(), r -> new Thread(r, "Connection Suggestion Provider")));
this.executorService = new ExecutorServiceShutdownWatchable<>(FailSafeExecutors.newSingleThreadExecutor("Connection Suggestion Provider"));
this.bindingSuggestionProvider = bindingSuggestionProvider;
this.clientFs = clientFs;
}
Expand Down Expand Up @@ -106,15 +103,15 @@ private void queueConnectionSuggestion(Set<String> listConfigScopeIds) {
if (!listConfigScopeIds.isEmpty()) {
var cancelMonitor = new SonarLintCancelMonitor();
cancelMonitor.watchForShutdown(executorService);
executorService.submit(() -> suggestConnectionForGivenScopes(listConfigScopeIds, cancelMonitor));
executorService.execute(() -> suggestConnectionForGivenScopes(listConfigScopeIds, cancelMonitor));
}
}

private void suggestConnectionForGivenScopes(Set<String> listOfFilesPerConfigScopeIds, SonarLintCancelMonitor cancelMonitor) {
LOG.debug("Computing connection suggestions");
var connectionSuggestionsByConfigScopeIds = new HashMap<String, List<ConnectionSuggestionDto>>();
var bindingSuggestionsForConfigScopeIds = new HashSet<String>();

for (var configScopeId : listOfFilesPerConfigScopeIds) {
var effectiveBinding = configRepository.getEffectiveBinding(configScopeId);
if (effectiveBinding.isPresent()) {
Expand All @@ -133,8 +130,8 @@ private void suggestConnectionForGivenScopes(Set<String> listOfFilesPerConfigSco
organization -> connectionSuggestionsByConfigScopeIds.computeIfAbsent(configScopeId, s -> new ArrayList<>())
.add(new ConnectionSuggestionDto(new SonarCloudConnectionSuggestionDto(organization, projectKey,
((BindingClueProvider.SonarCloudBindingClue) bindingClue).getRegion()),
bindingClue.isFromSharedConfiguration()))
), () -> bindingSuggestionsForConfigScopeIds.add(configScopeId));
bindingClue.isFromSharedConfiguration()))),
() -> bindingSuggestionsForConfigScopeIds.add(configScopeId));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,13 @@
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
import org.sonarsource.sonarlint.core.commons.ConnectionKind;
import org.sonarsource.sonarlint.core.commons.Version;
import org.sonarsource.sonarlint.core.commons.log.SonarLintLogger;
import org.sonarsource.sonarlint.core.commons.progress.ExecutorServiceShutdownWatchable;
import org.sonarsource.sonarlint.core.commons.progress.SonarLintCancelMonitor;
import org.sonarsource.sonarlint.core.commons.util.FailSafeExecutors;
import org.sonarsource.sonarlint.core.event.BindingConfigChangedEvent;
import org.sonarsource.sonarlint.core.event.ConfigurationScopesAddedEvent;
import org.sonarsource.sonarlint.core.repository.config.ConfigurationRepository;
Expand Down Expand Up @@ -64,8 +63,7 @@ public VersionSoonUnsupportedHelper(SonarLintRpcClient client, ConfigurationRepo
this.connectionRepository = connectionRepository;
this.connectionManager = connectionManager;
this.synchronizationService = synchronizationService;
this.executorService = new ExecutorServiceShutdownWatchable<>(new ThreadPoolExecutor(0, 1, 10L, TimeUnit.SECONDS,
new LinkedBlockingQueue<>(), r -> new Thread(r, "Version Soon Unsupported Helper")));
this.executorService = new ExecutorServiceShutdownWatchable<>(FailSafeExecutors.newSingleThreadExecutor("Version Soon Unsupported Helper"));
}

@EventListener
Expand Down Expand Up @@ -99,7 +97,7 @@ private void checkIfSoonUnsupportedOncePerConnection(Set<String> configScopeIds)
private void queueCheckIfSoonUnsupported(String connectionId, String configScopeId) {
var cancelMonitor = new SonarLintCancelMonitor();
cancelMonitor.watchForShutdown(executorService);
executorService.submit(() -> {
executorService.execute(() -> {
try {
var connection = connectionRepository.getConnectionById(connectionId);
if (connection != null && connection.getKind() == ConnectionKind.SONARQUBE) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@
import java.util.UUID;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.TimeUnit;
import java.util.function.Predicate;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -67,6 +66,7 @@
import org.sonarsource.sonarlint.core.commons.monitoring.MonitoringService;
import org.sonarsource.sonarlint.core.commons.progress.ProgressMonitor;
import org.sonarsource.sonarlint.core.commons.progress.SonarLintCancelMonitor;
import org.sonarsource.sonarlint.core.commons.util.FailSafeExecutors;
import org.sonarsource.sonarlint.core.event.BindingConfigChangedEvent;
import org.sonarsource.sonarlint.core.event.ConfigurationScopeRemovedEvent;
import org.sonarsource.sonarlint.core.event.ConfigurationScopesAddedEvent;
Expand Down Expand Up @@ -156,7 +156,7 @@ public class AnalysisService {
private final MonitoringService monitoringService;
private boolean automaticAnalysisEnabled;
private final Path esLintBridgeServerPath;
private final ScheduledExecutorService scheduledAnalysisExecutor = Executors.newSingleThreadScheduledExecutor(r -> new Thread(r, "SonarLint Analysis Executor"));
private final ExecutorService scheduledAnalysisExecutor = FailSafeExecutors.newSingleThreadExecutor("SonarLint Analysis Executor");

public AnalysisService(SonarLintRpcClient client, ConfigurationRepository configurationRepository, LanguageSupportRepository languageSupportRepository,
StorageService storageService, PluginsService pluginsService, RulesService rulesService, RulesRepository rulesRepository,
Expand Down Expand Up @@ -894,17 +894,15 @@ private void triggerAnalysisForOpenFiles() {
private UUID triggerForcedAnalysis(String configurationScopeId, List<URI> files, boolean hotspotsOnly) {
if (isReadyForAnalysis(configurationScopeId)) {
var analysisId = UUID.randomUUID();
scheduledAnalysisExecutor.submit(() -> {
analyze(new SonarLintCancelMonitor(), configurationScopeId, analysisId, files, Map.of(), System.currentTimeMillis(), true, hotspotsOnly);
return analysisId;
});
scheduledAnalysisExecutor
.execute(() -> analyze(new SonarLintCancelMonitor(), configurationScopeId, analysisId, files, Map.of(), System.currentTimeMillis(), true, hotspotsOnly));
}
LOG.debug("Skipping analysis for configuration scope {}. Not ready for analysis", configurationScopeId);
return null;
}

private void triggerAnalysis(String configurationScopeId, List<URI> files) {
scheduledAnalysisExecutor.submit(() -> {
scheduledAnalysisExecutor.execute(() -> {
if (shouldTriggerAutomaticAnalysis(configurationScopeId)) {
List<URI> filteredFiles = fileExclusionService.filterOutClientExcludedFiles(configurationScopeId, files);
analyze(new SonarLintCancelMonitor(), configurationScopeId, UUID.randomUUID(), filteredFiles, Map.of(), System.currentTimeMillis(), true, false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@
import java.util.Objects;
import java.util.concurrent.CancellationException;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;
import java.util.function.BiFunction;
import javax.annotation.Nullable;
import org.sonarsource.sonarlint.core.commons.log.SonarLintLogger;
import org.sonarsource.sonarlint.core.commons.progress.ExecutorServiceShutdownWatchable;
import org.sonarsource.sonarlint.core.commons.progress.SonarLintCancelMonitor;
import org.sonarsource.sonarlint.core.commons.util.FailSafeExecutors;

/**
* A cache with async computation of values, and supporting cancellation.
Expand Down Expand Up @@ -58,7 +58,7 @@ public SmartCancelableLoadingCache(String threadName, BiFunction<K, SonarLintCan
}

public SmartCancelableLoadingCache(String threadName, BiFunction<K, SonarLintCancelMonitor, V> valueComputer, @Nullable Listener<K, V> listener) {
this.executorService = new ExecutorServiceShutdownWatchable<>(Executors.newSingleThreadExecutor(r -> new Thread(r, threadName)));
this.executorService = new ExecutorServiceShutdownWatchable<>(FailSafeExecutors.newSingleThreadExecutor(threadName));
this.threadName = threadName;
this.valueComputer = valueComputer;
this.listener = listener;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@
import java.util.HashSet;
import java.util.Objects;
import java.util.concurrent.CancellationException;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import javax.annotation.CheckForNull;
Expand All @@ -39,6 +37,7 @@
import org.sonarsource.sonarlint.core.commons.log.SonarLintLogger;
import org.sonarsource.sonarlint.core.commons.progress.ExecutorServiceShutdownWatchable;
import org.sonarsource.sonarlint.core.commons.progress.SonarLintCancelMonitor;
import org.sonarsource.sonarlint.core.commons.util.FailSafeExecutors;
import org.sonarsource.sonarlint.core.repository.config.ConfigurationRepository;
import org.sonarsource.sonarlint.core.repository.connection.ConnectionConfigurationRepository;
import org.sonarsource.sonarlint.core.rpc.protocol.SonarLintRpcClient;
Expand Down Expand Up @@ -73,8 +72,7 @@ public RequestHandlerBindingAssistant(BindingSuggestionProvider bindingSuggestio
this.connectionConfigurationRepository = connectionConfigurationRepository;
this.configurationRepository = configurationRepository;
this.userTokenService = userTokenService;
this.executorService = new ExecutorServiceShutdownWatchable<>(new ThreadPoolExecutor(0, 1, 10L, TimeUnit.SECONDS,
new LinkedBlockingQueue<>(), r -> new Thread(r, "Show Issue or Hotspot Request Handler")));
this.executorService = new ExecutorServiceShutdownWatchable<>(FailSafeExecutors.newSingleThreadExecutor("Show Issue or Hotspot Request Handler"));
this.sonarCloudActiveEnvironment = sonarCloudActiveEnvironment;
this.repository = repository;
}
Expand All @@ -86,7 +84,7 @@ interface Callback {
void assistConnectionAndBindingIfNeededAsync(AssistCreatingConnectionParams connectionParams, String projectKey, String origin, Callback callback) {
var cancelMonitor = new SonarLintCancelMonitor();
cancelMonitor.watchForShutdown(executorService);
executorService.submit(() -> assistConnectionAndBindingIfNeeded(connectionParams, projectKey, origin, callback, cancelMonitor));
executorService.execute(() -> assistConnectionAndBindingIfNeeded(connectionParams, projectKey, origin, callback, cancelMonitor));
}

private void assistConnectionAndBindingIfNeeded(AssistCreatingConnectionParams connectionParams, String projectKey, String origin,
Expand All @@ -95,9 +93,8 @@ private void assistConnectionAndBindingIfNeeded(AssistCreatingConnectionParams c
LOG.debug("Assist connection and binding if needed for project {} and server {}", projectKey, serverUrl);
try {
var isSonarCloud = connectionParams.getConnectionParams().isRight();
var connectionsMatchingOrigin = isSonarCloud ?
connectionConfigurationRepository.findByOrganization(connectionParams.getConnectionParams().getRight().getOrganizationKey()) :
connectionConfigurationRepository.findByUrl(serverUrl);
var connectionsMatchingOrigin = isSonarCloud ? connectionConfigurationRepository.findByOrganization(connectionParams.getConnectionParams().getRight().getOrganizationKey())
: connectionConfigurationRepository.findByUrl(serverUrl);
if (connectionsMatchingOrigin.isEmpty()) {
startFullBindingProcess();
try {
Expand Down Expand Up @@ -130,8 +127,8 @@ private void assistConnectionAndBindingIfNeeded(AssistCreatingConnectionParams c
}

private String getServerUrl(AssistCreatingConnectionParams connectionParams) {
return connectionParams.getConnectionParams().isLeft() ? connectionParams.getConnectionParams().getLeft().getServerUrl() :
sonarCloudActiveEnvironment.getUri(SonarCloudRegion.valueOf(connectionParams.getConnectionParams().getRight().getRegion().name())).toString();
return connectionParams.getConnectionParams().isLeft() ? connectionParams.getConnectionParams().getLeft().getServerUrl()
: sonarCloudActiveEnvironment.getUri(SonarCloudRegion.valueOf(connectionParams.getConnectionParams().getRight().getRegion().name())).toString();
}

private AssistCreatingConnectionResponse assistCreatingConnectionAndWaitForRepositoryUpdate(
Expand Down Expand Up @@ -248,7 +245,6 @@ NewBinding assistBinding(String connectionId, boolean isSonarCloud, String proje
return new NewBinding(connectionId, response.getConfigurationScopeId());
}


static class NewBinding {
private final String connectionId;
private final String configurationScopeId;
Expand Down
Loading

0 comments on commit d69ade5

Please sign in to comment.