ics — fix possible deadlock on app exit
authorVladimir Krivosheev <vladimir.krivosheev@jetbrains.com>
Tue, 11 Aug 2020 05:41:57 +0000 (07:41 +0200)
committerintellij-monorepo-bot <intellij-monorepo-bot-no-reply@jetbrains.com>
Tue, 11 Aug 2020 05:58:44 +0000 (05:58 +0000)
GitOrigin-RevId: 960fa041d89aab910cf3c18e4b78a2f36bd1da31

platform/core-impl/src/com/intellij/openapi/application/ex/ApplicationEx.java
platform/platform-impl/src/com/intellij/openapi/application/impl/ApplicationImpl.java
plugins/settings-repository/src/IcsManager.kt
plugins/settings-repository/src/autoSync.kt
plugins/settings-repository/src/git/pull.kt
plugins/settings-repository/src/sync.kt

index e2c9a5385e7948f0a87a613355313b82ce40d32a..cc442087940262664595e5660296650d9615f3d8 100644 (file)
@@ -226,4 +226,9 @@ public interface ApplicationEx extends Application {
   default void runIntendedWriteActionOnCurrentThread(@NotNull Runnable action) {
     action.run();
   }
+
+  @ApiStatus.Internal
+  default boolean isExitInProgress() {
+    return false;
+  }
 }
\ No newline at end of file
index 954b56aec266e1f8dbf621a42c22487b67ad860f..22a0fb926a5638781cc2e742c158c437a1674537 100644 (file)
@@ -583,6 +583,11 @@ public class ApplicationImpl extends ComponentManagerImpl implements Application
     }
   }
 
+  @Override
+  public final boolean isExitInProgress() {
+    return myExitInProgress;
+  }
+
   private void doExit(int flags, boolean restart, String[] beforeRestart) {
     boolean force = BitUtil.isSet(flags, FORCE_EXIT);
     try {
index 382e206b2abe6d32e0d79b1108450282dbe65982..f53034e2fc2cd6e67874743620363f8f3ba43c5a 100644 (file)
@@ -7,6 +7,7 @@ import com.intellij.ide.AppLifecycleListener
 import com.intellij.ide.ApplicationLoadListener
 import com.intellij.openapi.application.Application
 import com.intellij.openapi.application.ApplicationManager
+import com.intellij.openapi.application.ex.ApplicationManagerEx
 import com.intellij.openapi.components.RoamingType
 import com.intellij.openapi.components.stateStore
 import com.intellij.openapi.diagnostic.logger
@@ -136,7 +137,7 @@ class IcsManager @JvmOverloads constructor(dir: Path, val schemeManagerFactory:
 
     app.stateStore.storageManager.addStreamProvider(ApplicationLevelProvider())
 
-    val messageBusConnection = app.messageBus.connect()
+    val messageBusConnection = app.messageBus.simpleConnect()
     messageBusConnection.subscribe(AppLifecycleListener.TOPIC, object : AppLifecycleListener {
       override fun appWillBeClosed(isRestart: Boolean) {
         autoSyncManager.autoSync(true)
@@ -148,7 +149,9 @@ class IcsManager @JvmOverloads constructor(dir: Path, val schemeManagerFactory:
       }
 
       override fun projectClosed(project: Project) {
-        autoSyncManager.autoSync()
+        if (!ApplicationManagerEx.getApplicationEx().isExitInProgress) {
+          autoSyncManager.autoSync()
+        }
       }
     })
   }
index 8b149612cd6794a388086108648087a64da60000..c005918fe8e05e186dc646c776b0b1c520fa3f5e 100644 (file)
@@ -5,7 +5,6 @@ import com.intellij.configurationStore.ComponentStoreImpl
 import com.intellij.notification.Notification
 import com.intellij.notification.Notifications
 import com.intellij.openapi.application.AppUIExecutor
-import com.intellij.openapi.application.Application
 import com.intellij.openapi.application.ApplicationManager
 import com.intellij.openapi.application.ModalityState
 import com.intellij.openapi.application.impl.coroutineDispatchingContext
@@ -80,15 +79,33 @@ internal class AutoSyncManager(private val icsManager: IcsManager) {
       }
     }
 
-    val app = ApplicationManager.getApplication()
-
     if (onAppExit) {
-      runBlocking {
-        sync(app, onAppExit)
+      // called on final confirmed exit - no need to restore enabled state
+      enabled = false
+      catchAndLog {
+        runBlocking {
+          icsManager.runInAutoCommitDisabledMode {
+            val repositoryManager = icsManager.repositoryManager
+            val hasUpstream = repositoryManager.hasUpstream()
+            if (hasUpstream && !repositoryManager.canCommit()) {
+              LOG.warn("Auto sync skipped: repository is not committable")
+              return@runInAutoCommitDisabledMode
+            }
+
+            // on app exit fetch and push only if there are commits to push
+            // if no upstream - just update cloud schemes
+            if (hasUpstream && !repositoryManager.commit() && repositoryManager.getAheadCommitsCount() == 0 && icsManager.readOnlySourcesManager.repositories.isEmpty()) {
+              return@runInAutoCommitDisabledMode
+            }
+
+            // use explicit progress task to sync on app exit to make it clear why app is not exited immediately
+            icsManager.syncManager.sync(SyncType.MERGE, onAppExit = true)
+          }
+        }
       }
       return
     }
-    else if (app.isDisposed) {
+    else if (ApplicationManager.getApplication().isDisposed) {
       // will be handled by applicationExiting listener
       return
     }
@@ -97,7 +114,11 @@ internal class AutoSyncManager(private val icsManager: IcsManager) {
       try {
         // to ensure that repository will not be in uncompleted state and changes will be pushed
         ShutDownTracker.getInstance().registerStopperThread(Thread.currentThread())
-        sync(app, onAppExit)
+        catchAndLog {
+          icsManager.runInAutoCommitDisabledMode {
+            doSync()
+          }
+        }
       }
       finally {
         autoSyncFuture = null
@@ -106,15 +127,8 @@ internal class AutoSyncManager(private val icsManager: IcsManager) {
     }
   }
 
-  private suspend fun sync(app: Application, onAppExit: Boolean) {
-    catchAndLog {
-      icsManager.runInAutoCommitDisabledMode {
-        doSync(app, onAppExit)
-      }
-    }
-  }
-
-  private suspend fun doSync(app: Application, onAppExit: Boolean) {
+  private suspend fun doSync() {
+    val app = ApplicationManager.getApplication()
     val repositoryManager = icsManager.repositoryManager
     val hasUpstream = repositoryManager.hasUpstream()
     if (hasUpstream && !repositoryManager.canCommit()) {
@@ -122,40 +136,27 @@ internal class AutoSyncManager(private val icsManager: IcsManager) {
       return
     }
 
-    // on app exit fetch and push only if there are commits to push
-    if (onAppExit) {
-      // if no upstream - just update cloud schemes
-      if (hasUpstream && !repositoryManager.commit() && repositoryManager.getAheadCommitsCount() == 0 && icsManager.readOnlySourcesManager.repositories.isEmpty()) {
-        return
-      }
+    // update read-only sources at first (because contain scheme - to ensure that some scheme will exist when it will be set as current by some setting)
+    updateCloudSchemes(icsManager)
 
-      // use explicit progress task to sync on app exit to make it clear why app is not exited immediately
-      icsManager.syncManager.sync(SyncType.MERGE, onAppExit = true)
+    if (!hasUpstream) {
       return
     }
 
-    // update read-only sources at first (because contain scheme - to ensure that some scheme will exist when it will be set as current by some setting)
-    updateCloudSchemes(icsManager)
-
-    if (hasUpstream) {
-      val updater = repositoryManager.fetch()
-      // we merge in EDT non-modal to ensure that new settings will be properly applied
-      withContext(AppUIExecutor.onUiThread(ModalityState.NON_MODAL).coroutineDispatchingContext()) {
-        catchAndLog {
-          val updateResult = updater.merge()
-          if (!onAppExit &&
-              !app.isDisposed &&
-              updateResult != null &&
-              updateStoragesFromStreamProvider(icsManager, app.stateStore as ComponentStoreImpl, updateResult)) {
-            // force to avoid saveAll & confirmation
-            app.exit(true, true, true)
-          }
+    val updater = repositoryManager.fetch()
+    // we merge in EDT non-modal to ensure that new settings will be properly applied
+    withContext(AppUIExecutor.onUiThread(ModalityState.NON_MODAL).coroutineDispatchingContext()) {
+      catchAndLog {
+        val updateResult = updater.merge()
+        if (!app.isDisposed && updateResult != null && updateStoragesFromStreamProvider(icsManager, app.stateStore as ComponentStoreImpl, updateResult)) {
+          // force to avoid saveAll & confirmation
+          app.exit(true, true, true)
         }
       }
+    }
 
-      if (!updater.definitelySkipPush) {
-        repositoryManager.push()
-      }
+    if (!updater.definitelySkipPush) {
+      repositoryManager.push()
     }
   }
 }
@@ -164,7 +165,8 @@ internal inline fun catchAndLog(asWarning: Boolean = false, runnable: () -> Unit
   try {
     runnable()
   }
-  catch (e: ProcessCanceledException) { }
+  catch (e: ProcessCanceledException) {
+  }
   catch (e: Throwable) {
     if (asWarning || e is AuthenticationException || e is NoRemoteRepositoryException) {
       LOG.warn(e)
index cfc7d42a14badca98b5a6922db67c55f8d9bcad2..2d0692f6e6edd4aa9b10d28e88288ce2bf2d9987 100644 (file)
@@ -1,4 +1,4 @@
-// Copyright 2000-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
+// Copyright 2000-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
 package org.jetbrains.settingsRepository.git
 
 import com.intellij.openapi.diagnostic.debug
@@ -334,7 +334,7 @@ internal suspend fun resolveUnmergedConflicts(repository: Repository) {
   val conflicts = LinkedHashMap<String, Array<ByteArray?>>()
   repository.newObjectReader().use { reader ->
     val dirCache = repository.readDirCache()
-    for (i in 0..(dirCache.entryCount - 1)) {
+    for (i in 0 until dirCache.entryCount) {
       val entry = dirCache.getEntry(i)
       if (!entry.isMerged) {
         conflicts.getOrPut(entry.pathString) { arrayOfNulls(3) }[entry.stage - 1] = reader.open(entry.objectId, Constants.OBJ_BLOB).cachedBytes
index 99dd85b228c8509c0b5fc5ad0f2b7ea09d0f7837..798728c01d13367c72b2994af90ff62c35fae8a2 100644 (file)
@@ -1,10 +1,7 @@
 // Copyright 2000-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
 package org.jetbrains.settingsRepository
 
-import com.intellij.configurationStore.ComponentStoreImpl
-import com.intellij.configurationStore.StateStorageManagerImpl
-import com.intellij.configurationStore.XmlElementStorage
-import com.intellij.configurationStore.askToRestart
+import com.intellij.configurationStore.*
 import com.intellij.configurationStore.schemeManager.SchemeManagerImpl
 import com.intellij.openapi.application.AppUIExecutor
 import com.intellij.openapi.application.ApplicationManager
@@ -29,7 +26,9 @@ internal class SyncManager(private val icsManager: IcsManager, private val autoS
   private suspend fun runSyncTask(onAppExit: Boolean, project: Project?, task: suspend (indicator: ProgressIndicator) -> Unit) {
     icsManager.runInAutoCommitDisabledMode {
       if (!onAppExit) {
-        ApplicationManager.getApplication()!!.saveSettings()
+        runInAutoSaveDisabledMode {
+          saveSettings(ApplicationManager.getApplication(), false)
+        }
       }
 
       try {
@@ -176,8 +175,12 @@ internal fun updateCloudSchemes(icsManager: IcsManager, indicator: ProgressIndic
 }
 
 
-internal suspend fun updateStoragesFromStreamProvider(icsManager: IcsManager, store: ComponentStoreImpl, updateResult: UpdateResult, reloadAllSchemes: Boolean = false): Boolean {
-  val (changed, deleted) = (store.storageManager as StateStorageManagerImpl).getCachedFileStorages(updateResult.changed, updateResult.deleted, ::toIdeaPath)
+internal suspend fun updateStoragesFromStreamProvider(icsManager: IcsManager,
+                                                      store: ComponentStoreImpl,
+                                                      updateResult: UpdateResult,
+                                                      reloadAllSchemes: Boolean = false): Boolean {
+  val (changed, deleted) = (store.storageManager as StateStorageManagerImpl).getCachedFileStorages(updateResult.changed,
+                                                                                                   updateResult.deleted, ::toIdeaPath)
 
   val schemeManagersToReload = SmartList<SchemeManagerImpl<*, *>>()
   icsManager.schemeManagerFactory.value.process {