Merge pull request #2086 from digma-ai/ensure-no-read-access-in-backgroundable

ensue no read access in Backgroundable Closes #2084

Author: shalom938

ide-common/src/main/java/org/digma/intellij/plugin/analytics/AnalyticsServiceStarter.java

@@ -12,6 +12,6 @@ public class AnalyticsServiceStarter implements StartupActivity.DumbAware {
 
     @Override
     public void runActivity(@NotNull Project project) {
-        Backgroundable.ensureBackground(project, "initializing analytics service", () -> AnalyticsService.getInstance(project));
+        Backgroundable.ensureBackgroundWithoutReadAccess(project, "initializing analytics service", () -> AnalyticsService.getInstance(project));
     }
 }

ide-common/src/main/java/org/digma/intellij/plugin/analytics/Environment.java

@@ -98,7 +98,7 @@ public void setCurrent(@NotNull String newEnv, @Nullable Runnable taskToRunAfter
                 }
             };
 
-            Backgroundable.ensureBackground(project, "Digma: environment changed " + newEnv, task);
+            Backgroundable.ensureBackgroundWithoutReadAccess(project, "Digma: environment changed " + newEnv, task);
 
         } catch (Throwable e) {
             ErrorReporter.getInstance().reportError(project, "Environment.setCurrent", e);
@@ -123,7 +123,7 @@ public List<String> getEnvironmentsNames() {
     public void refreshNowOnBackground() {
 
         Log.log(LOGGER::trace, "Refreshing Environments on background thread.");
-        Backgroundable.ensureBackground(project, "Refreshing Environments", () -> {
+        Backgroundable.ensureBackgroundWithoutReadAccess(project, "Refreshing Environments", () -> {
             envChangeLock.lock();
             try {
                 //run both refreshEnvironments and updateCurrentEnv under same lock

ide-common/src/main/java/org/digma/intellij/plugin/common/Backgroundable.java

@@ -2,15 +2,14 @@
 
 import com.intellij.openapi.application.ApplicationManager;
 import com.intellij.openapi.diagnostic.Logger;
-import com.intellij.openapi.progress.ProgressIndicator;
-import com.intellij.openapi.progress.Task;
+import com.intellij.openapi.progress.*;
 import com.intellij.openapi.project.Project;
+import com.intellij.util.concurrency.FutureResult;
 import org.digma.intellij.plugin.errorreporting.ErrorReporter;
 import org.digma.intellij.plugin.log.Log;
 import org.jetbrains.annotations.NotNull;
 
-import java.util.concurrent.Callable;
-import java.util.concurrent.Future;
+import java.util.concurrent.*;
 
 public class Backgroundable {
 
@@ -20,6 +19,7 @@ private Backgroundable() {
     }
 
 
+    //it's better to use ensureBackgroundWithoutReadAccess
     public static void ensureBackground(Project project, String name, Runnable task) {
 
         Log.log(LOGGER::trace, "Request to call task '{}'", name);
@@ -39,6 +39,25 @@ public void run(@NotNull ProgressIndicator indicator) {
     }
 
 
+    public static void ensureBackgroundWithoutReadAccess(Project project, String name, Runnable task) {
+
+        Log.log(LOGGER::trace, "Request to call task '{}'", name);
+
+        if (EDT.isEdt() || ReadActions.isReadAccessAllowed()) {
+            Log.log(LOGGER::trace, "Executing task '{}' in background thread", name);
+            new Task.Backgroundable(project, name) {
+                @Override
+                public void run(@NotNull ProgressIndicator indicator) {
+                    runWithErrorReporting(project, name, task);
+                }
+            }.queue();
+        } else {
+            Log.log(LOGGER::trace, "Executing task '{}' in current thread", name);
+            runWithErrorReporting(project, name, task);
+        }
+    }
+
+
 
 
     public static void runInNewBackgroundThread(Project project, String name, Runnable task) {
@@ -52,6 +71,7 @@ public void run(@NotNull ProgressIndicator indicator) {
 
     }
 
+    //it's better to use ensurePooledThreadWithoutReadAccess
     public static void ensurePooledThread(@NotNull Runnable action){
         if (EDT.isEdt()) {
             executeOnPooledThread(action);
@@ -60,7 +80,24 @@ public static void ensurePooledThread(@NotNull Runnable action){
         }
     }
 
+    public static void ensurePooledThreadWithoutReadAccess(@NotNull Runnable action) {
+        if (EDT.isEdt() || ReadActions.isReadAccessAllowed()) {
+            executeOnPooledThread(action);
+        } else {
+            runWithErrorReporting(action);
+        }
+    }
+
+    public static <T> Future<T> ensurePooledThreadWithoutReadAccess(@NotNull Callable<T> action) {
+        if (EDT.isEdt() || ReadActions.isReadAccessAllowed()) {
+            return executeOnPooledThread(action);
+        } else {
+            return runWithErrorReporting(action);
+        }
+    }
+
 
+    //can use the future to wait for the thread to finish if necessary
     public static Future<?> executeOnPooledThread(@NotNull Runnable action) {
         return ApplicationManager.getApplication().executeOnPooledThread(() -> runWithErrorReporting(action));
     }
@@ -85,7 +122,7 @@ private static void runWithErrorReporting(Project project, String name, Runnable
             task.run();
         } catch (Throwable e) {
             Log.warnWithException(LOGGER, e, "Exception in task {}",name);
-            ErrorReporter.getInstance().reportError(project, "Backgroundable.runWithErrorReporting." + name, e);
+            ErrorReporter.getInstance().reportError(project, "Backgroundable.runWithErrorReporting(Project,Runnable)" + name, e);
         }
     }
 
@@ -94,7 +131,18 @@ private static void runWithErrorReporting(Runnable task) {
             task.run();
         } catch (Throwable e) {
             Log.warnWithException(LOGGER, e, "Exception in action");
-            ErrorReporter.getInstance().reportError("Backgroundable.runWithErrorReporting", e);
+            ErrorReporter.getInstance().reportError("Backgroundable.runWithErrorReporting(Runnable)", e);
+        }
+    }
+
+    private static <T> Future<T> runWithErrorReporting(@NotNull Callable<T> action) {
+
+        try {
+            return new FutureResult<>(action.call());
+        } catch (Exception e) {
+            Log.warnWithException(LOGGER, e, "Exception in action");
+            ErrorReporter.getInstance().reportError("Backgroundable.runWithErrorReporting(Callable)", e);
+            return new FutureResult<>();
         }
     }
 

ide-common/src/main/kotlin/org/digma/intellij/plugin/codelens/CodeLensService.kt

@@ -221,7 +221,7 @@ class CodeLensService(private val project: Project) : Disposable {
                         selectedEditor?.caretModel?.moveToOffset(it.textOffset)
                     }
                 }
-                Backgroundable.ensurePooledThread {
+                Backgroundable.ensurePooledThreadWithoutReadAccess {
                     if (lens.scopeCodeObjectId.startsWith("span:")) {
                         ScopeManager.getInstance(project).changeScope(SpanScope(lens.scopeCodeObjectId))
                     }

ide-common/src/main/kotlin/org/digma/intellij/plugin/progress/ProgressUtils.kt

@@ -18,7 +18,7 @@ fun runInvisibleBackgroundTaskInProgressWithRetry(task: RetryableTask.Invisible)
     if (task.reuseCurrentThread) {
         //even if reuseCurrentThread is true , still make sure it's a background thread. these tasks
         // are not meant to run on EDT.
-        Backgroundable.ensurePooledThread {
+        Backgroundable.ensurePooledThreadWithoutReadAccess {
             invisibleRetryableTask.run()
         }
     } else {

jvm-common/src/main/kotlin/org/digma/intellij/plugin/idea/navigation/AbstractNavigationDiscovery.kt

@@ -286,7 +286,7 @@ abstract class AbstractNavigationDiscovery(protected val project: Project) : Dis
             return
         }
 
-        Backgroundable.ensurePooledThread {
+        Backgroundable.ensurePooledThreadWithoutReadAccess {
             if (virtualFile != null) {
                 buildLock.lock()
                 try {
@@ -304,7 +304,7 @@ abstract class AbstractNavigationDiscovery(protected val project: Project) : Dis
             return
         }
 
-        Backgroundable.ensurePooledThread {
+        Backgroundable.ensurePooledThreadWithoutReadAccess {
             buildLock.lock()
             try {
                 removeDiscoveryForPath(path)

python/src/main/java/org/digma/intellij/plugin/psi/python/PythonSpanNavigationProvider.java

@@ -226,7 +226,7 @@ public void fileDeleted(VirtualFile virtualFile) {
             return;
         }
 
-        Backgroundable.ensurePooledThread(() -> {
+        Backgroundable.ensurePooledThreadWithoutReadAccess(() -> {
             if (virtualFile != null) {
                 buildSpansLock.lock();
                 try {

src/main/kotlin/org/digma/intellij/plugin/ui/navigation/CodeButtonCaretContextService.kt

@@ -55,7 +55,7 @@ class CodeButtonCaretContextService(private val project: Project) : CaretContext
         }
 
 
-        Backgroundable.ensurePooledThread {
+        Backgroundable.ensurePooledThreadWithoutReadAccess {
             Log.log(logger::trace, "Executing contextChanged in background for {}", methodUnderCaret.id)
             val stopWatch = StopWatch.createStarted()
             try {

src/main/kotlin/org/digma/intellij/plugin/ui/notificationcenter/EventsNotificationsService.kt

@@ -207,7 +207,7 @@ class GoToCodeObjectInsightsAction(
         val environmentsSupplier: EnvironmentsSupplier = AnalyticsService.getInstance(project).environment
 
         val runnable = Runnable {
-            Backgroundable.ensurePooledThread {
+            Backgroundable.ensurePooledThreadWithoutReadAccess {
                 ScopeManager.getInstance(project).changeScope(SpanScope(codeObjectId))
             }
         }

src/main/kotlin/org/digma/intellij/plugin/ui/notificationcenter/RegistrationRequestNotifications.kt

@@ -143,7 +143,7 @@ class RegisterAction(
 
     private fun openRegistrationDialog(project: Project) {
         //this is called on EDT, start background and release the EDT thread
-        Backgroundable.ensurePooledThread {
+        Backgroundable.ensurePooledThreadWithoutReadAccess {
             project.service<RecentActivityService>().openRegistrationDialog()
         }
     }