Merge pull request #2405 from digma-ai/fix-no-insights-yet-notification

fix disposing in scheduled tasks Closes #2400

Author: shalom938

ide-common/src/main/kotlin/org/digma/intellij/plugin/scheduling/schedulers.kt

@@ -317,13 +317,25 @@ fun Disposable.disposingPeriodicTask(name: String, startupDelay: Long, period: L
         return false
     }
 
-    Disposer.register(this) {
-        Log.log(logger::trace, "disposing periodic task {}", name)
+    //it may happen that 'this' is already disposed before registering a disposer.
+    //for example tasks that dispose themselves. see for example startNoInsightsYetNotificationTimer,
+    // in a matter of microseconds it may happen that the task will run and dispose itself before we had a chance to register a disposer,
+    //if that happens the task will never be canceled.
+    //or 'this' may be disposed very quickly by some other component.
+    //in any case, if Disposer.register fails for any reason then cancel the task immediately, otherwise the task will never be canceled.
+    //we send an error event, although it is not really an error because it is probably an ok flow, but maybe we want to know about
+    // these cases and improve if necessary.
+    try {
+        Disposer.register(this) {
+            Log.log(logger::trace, "disposing periodic task {}", name)
+            future.cancel(true)
+        }
+    } catch (e: Throwable) {
         future.cancel(true)
+        Log.warnWithException(logger, e, "could not register disposer for task {}", name)
+        ErrorReporter.getInstance().reportError("org.digma.scheduler.disposingPeriodicTask", e, mapOf("task.name" to name))
     }
     return true
-
-
 }
 
 /**
@@ -337,7 +349,7 @@ fun Disposable.disposingOneShotDelayedTask(name: String, delay: Long, block: ()
     consider this:
     calling disposingOneShotDelayedTask on a Disposable ,lets say a project service. and we would just register
     a child disposable on the project service to cancel the task.
-    if the project is closed before the task is executed the parent disposable, the service, will be disposed with all its children
+    if the project is closed before the task is executed, the parent disposable, the service, will be disposed with all its children
     and the task will be canceled.
     but if the task did execute and the project service was not disposed yet, we are left with a disposable child that will be disposed
     only when the service is disposed.
@@ -347,14 +359,14 @@ fun Disposable.disposingOneShotDelayedTask(name: String, delay: Long, block: ()
     the solution:
     calling disposingOneShotDelayedTask on a Disposable project service. the parent.
     we create here a new disposable,the child, register it as child of the parent.
-    register the task on the child disposable and in disposingOneShotDelayedTask0 we register a child for the child.
+    register the task on the child disposable and in disposingOneShotDelayedTaskImpl we register a child for the child.
     if the parent is disposed before the task is executed, the first child will be disposed, its child will be disposed and the task
     will be canceled.
     if the task executed and completed, the task itself disposes the first child, which will dispose its child. and now the parent
     doesn't have a leftover child.
 
-    Notice that we execute disposingOneShotDelayedTask0 on the child disposable, its private and should only be used internally.
-    in disposingOneShotDelayedTask0 when the task completes it disposes this in the finally block.
+    Notice that we execute disposingOneShotDelayedTaskImpl on the child disposable, its private and should only be used internally.
+    in disposingOneShotDelayedTaskImpl when the task completes it disposes 'this' in the finally block.
     see unit tests:
     testDisposingOneShotDelayedTaskParentDisposableHasNoChildren
     testDisposingOneShotDelayedTaskCanceledParentDisposableHasNoChildren
@@ -404,14 +416,26 @@ private fun Disposable.disposingOneShotDelayedTaskImpl(name: String, delay: Long
         return false
     }
 
-
-    //this will run when parent disposable is disposed which will dispose this disposable and its children,if not disposed already.
-    Disposer.register(this) {
-        Log.log(logger::trace, "disposing delayed task {}", name)
+    //it may happen that 'this' is already disposed before registering a disposer.
+    //for example tasks that dispose themselves. see for example startNoInsightsYetNotificationTimer,
+    // in a matter of microseconds it may happen that the task will run and dispose itself before we had a chance to register a disposer,
+    //if that happens the task will never be canceled.
+    //or 'this' may be disposed very quickly by some other component.
+    //in any case, if Disposer.register fails for any reason then cancel the task immediately, otherwise the task will never be canceled.
+    //we send an error event, although it is not really an error because it is probably an ok flow, but maybe we want to know about
+    // these cases and improve if necessary.
+    try {
+        Disposer.register(this) {
+            Log.log(logger::trace, "disposing delayed task {}", name)
+            future.cancel(true)
+        }
+    } catch (e: Throwable) {
         future.cancel(true)
+        Log.warnWithException(logger, e, "could not register disposer for task {}", name)
+        ErrorReporter.getInstance().reportError("org.digma.scheduler.disposingPeriodicTask", e, mapOf("task.name" to name))
     }
-
     return true
+
 }
 
 /**

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

@@ -15,7 +15,7 @@ import org.digma.intellij.plugin.scheduling.disposingPeriodicTask
 import java.time.Instant
 import kotlin.time.Duration.Companion.minutes
 
-
+//parentDisposable is the service
 fun startNoInsightsYetNotificationTimer(parentDisposable: Disposable) {
 
     if (service<PersistenceService>().isFirstInsightReceived()) {
@@ -44,9 +44,7 @@ private fun scheduleShowNotification(parentDisposable: Disposable, firstConnecti
         firstConnectionTime
     )
 
-    val disposable = Disposer.newDisposable()
-    Disposer.register(parentDisposable, disposable)
-    disposable.disposingOneShotDelayedTask("NoInsightsYetNotificationTimer.showNoInsightsYetNotification", 30.minutes.inWholeMilliseconds) {
+    parentDisposable.disposingOneShotDelayedTask("NoInsightsYetNotificationTimer.showNoInsightsYetNotification", 30.minutes.inWholeMilliseconds) {
         if (!service<PersistenceService>().isFirstInsightReceived()) {
             Log.log(AppNotificationCenter.logger::info, "in NoInsightsYetNotificationTimer, showing notification")
             showNoInsightsYetNotification()