vcs: fix batch file change operations handling

platform/vcs-impl/src/com/intellij/openapi/vcs/ex/LineStatusTracker.kt

@@ -16,6 +16,7 @@
 package com.intellij.openapi.vcs.ex
 
 import com.intellij.diff.util.DiffUtil
+import com.intellij.diff.util.Side
 import com.intellij.ide.GeneralSettings
 import com.intellij.openapi.actionSystem.AnAction
 import com.intellij.openapi.actionSystem.IdeActions
@@ -30,6 +31,7 @@ import com.intellij.openapi.fileTypes.FileType
 import com.intellij.openapi.project.Project
 import com.intellij.openapi.vcs.changes.VcsDirtyScopeManager
 import com.intellij.openapi.vfs.VirtualFile
+import org.jetbrains.annotations.CalledInAny
 import org.jetbrains.annotations.CalledInAwt
 import java.awt.Graphics
 import java.awt.Point
@@ -131,4 +133,16 @@ abstract class LineStatusTracker<R : Range> constructor(override val project: Pr
       }
     }
   }
+
+  @CalledInAny
+  internal fun freeze() {
+    documentTracker.freeze(Side.LEFT)
+    documentTracker.freeze(Side.RIGHT)
+  }
+
+  @CalledInAwt
+  internal fun unfreeze() {
+    documentTracker.unfreeze(Side.LEFT)
+    documentTracker.unfreeze(Side.RIGHT)
+  }
 }

platform/vcs-impl/src/com/intellij/openapi/vcs/ex/PartialLocalLineStatusTracker.kt

@@ -16,7 +16,6 @@
 package com.intellij.openapi.vcs.ex
 
 import com.intellij.diff.util.Side
-import com.intellij.ide.file.BatchFileChangeListener
 import com.intellij.openapi.Disposable
 import com.intellij.openapi.actionSystem.DefaultActionGroup
 import com.intellij.openapi.actionSystem.Separator
@@ -54,7 +53,6 @@ import java.awt.Graphics
 import java.awt.Point
 import java.lang.ref.WeakReference
 import java.util.*
-import java.util.concurrent.atomic.AtomicInteger
 import javax.swing.JComponent
 import javax.swing.JPanel
 import kotlin.collections.HashSet
@@ -77,7 +75,6 @@ class PartialLocalLineStatusTracker(project: Project,
   private var lastKnownTrackerChangeListId: String? = null
   private val affectedChangeLists = HashSet<String>()
 
-  private val batchChangeTaskCounter: AtomicInteger = AtomicInteger()
   private var hasUndoInCommand: Boolean = false
 
   private var shouldInitializeWithExcludedFromCommit: Boolean = false
@@ -88,9 +85,6 @@ class PartialLocalLineStatusTracker(project: Project,
     defaultMarker = ChangeListMarker(changeListManager.defaultChangeList)
     affectedChangeLists.add(defaultMarker.changelistId)
 
-    val connection = application.messageBus.connect(disposable)
-    connection.subscribe(BatchFileChangeListener.TOPIC, MyBatchFileChangeListener())
-
     document.addDocumentListener(MyUndoDocumentListener(), disposable)
     CommandProcessor.getInstance().addCommandListener(MyUndoCommandListener(), disposable)
     Disposer.register(disposable, Disposable { dropExistingUndoActions() })
@@ -302,27 +296,6 @@ class PartialLocalLineStatusTracker(project: Project,
     undoableActions.add(action)
   }
 
-  private inner class MyBatchFileChangeListener : BatchFileChangeListener {
-    override fun batchChangeStarted(eventProject: Project, activityName: String?) {
-      if (eventProject != project) return
-      if (batchChangeTaskCounter.getAndIncrement() == 0) {
-        documentTracker.freeze(Side.LEFT)
-        documentTracker.freeze(Side.RIGHT)
-      }
-    }
-
-    override fun batchChangeCompleted(eventProject: Project) {
-      if (eventProject != project) return
-      application.invokeLater(
-        {
-          if (batchChangeTaskCounter.decrementAndGet() == 0) {
-            documentTracker.unfreeze(Side.LEFT)
-            documentTracker.unfreeze(Side.RIGHT)
-          }
-        }, ModalityState.any())
-    }
-  }
-
   private inner class PartialDocumentTrackerHandler : LineStatusTrackerBase<LocalRange>.MyDocumentTrackerHandler() {
     override fun onRangeAdded(block: Block) {
       super.onRangeAdded(block)

platform/vcs-impl/src/com/intellij/openapi/vcs/impl/LineStatusTrackerManager.kt

@@ -18,6 +18,7 @@ package com.intellij.openapi.vcs.impl
 import com.google.common.collect.HashMultiset
 import com.google.common.collect.Multiset
 import com.intellij.icons.AllIcons
+import com.intellij.ide.file.BatchFileChangeListener
 import com.intellij.notification.Notification
 import com.intellij.notification.NotificationAction
 import com.intellij.notification.NotificationType
@@ -89,6 +90,7 @@ class LineStatusTrackerManager(
 
   private var partialChangeListsEnabled = VcsApplicationSettings.getInstance().ENABLE_PARTIAL_CHANGELISTS && Registry.`is`("vcs.enable.partial.changelists")
   private val documentsInDefaultChangeList = HashSet<Document>()
+  private var batchChangeTaskCounter: Int = 0
 
   private val filesWithDamagedInactiveRanges = HashSet<VirtualFile>()
   private val fileStatesAwaitingRefresh = HashMap<VirtualFile, PartialLocalLineStatusTracker.State>()
@@ -115,8 +117,11 @@ class LineStatusTrackerManager(
 
       application.addApplicationListener(MyApplicationListener(), disposable)
 
-      val busConnection = project.messageBus.connect(disposable)
-      busConnection.subscribe(LineStatusTrackerSettingListener.TOPIC, MyLineStatusTrackerSettingListener())
+      val projectConnection = project.messageBus.connect(disposable)
+      projectConnection.subscribe(LineStatusTrackerSettingListener.TOPIC, MyLineStatusTrackerSettingListener())
+
+      val appConnection = application.messageBus.connect(disposable)
+      appConnection.subscribe(BatchFileChangeListener.TOPIC, MyBatchFileChangeListener())
 
       val fsManager = FileStatusManager.getInstance(project)
       fsManager.addFileStatusListener(MyFileStatusListener(), disposable)
@@ -417,6 +422,10 @@ class LineStatusTrackerManager(
       refreshTracker(tracker)
       eventDispatcher.multicaster.onTrackerAdded(tracker)
 
+      if (batchChangeTaskCounter > 0) {
+        tracker.freeze()
+      }
+
       log("Tracker installed", virtualFile)
       return tracker
     }
@@ -748,6 +757,46 @@ class LineStatusTrackerManager(
     }
   }
 
+  private inner class MyBatchFileChangeListener : BatchFileChangeListener {
+    override fun batchChangeStarted(eventProject: Project, activityName: String?) {
+      if (eventProject != project) return
+      runReadAction {
+        synchronized(LOCK) {
+          if (batchChangeTaskCounter == 0) {
+            for (data in trackers.values) {
+              try {
+                data.tracker.freeze()
+              }
+              catch (e: Throwable) {
+                LOG.error(e)
+              }
+            }
+          }
+          batchChangeTaskCounter++
+        }
+      }
+    }
+
+    override fun batchChangeCompleted(eventProject: Project) {
+      if (eventProject != project) return
+      runInEdt(ModalityState.any()) {
+        synchronized(LOCK) {
+          batchChangeTaskCounter--
+          if (batchChangeTaskCounter == 0) {
+            for (data in trackers.values) {
+              try {
+                data.tracker.unfreeze()
+              }
+              catch (e: Throwable) {
+                LOG.error(e)
+              }
+            }
+          }
+        }
+      }
+    }
+  }
+
 
   private fun shouldBeUpdated(oldInfo: ContentInfo?, newInfo: ContentInfo): Boolean {
     if (oldInfo == null) return true

platform/vcs-tests/testSrc/com/intellij/openapi/vcs/BaseLineStatusTrackerManagerTest.kt

@@ -1,12 +1,14 @@
 // Copyright 2000-2018 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 com.intellij.openapi.vcs
 
+import com.intellij.ide.file.BatchFileChangeListener
 import com.intellij.openapi.application.ApplicationManager
 import com.intellij.openapi.application.runWriteAction
 import com.intellij.openapi.command.CommandProcessor
 import com.intellij.openapi.editor.Document
 import com.intellij.openapi.fileEditor.FileDocumentManager
 import com.intellij.openapi.progress.ProgressIndicator
+import com.intellij.openapi.progress.util.BackgroundTaskUtil
 import com.intellij.openapi.project.Project
 import com.intellij.openapi.vcs.BaseLineStatusTrackerTestCase.Companion.parseInput
 import com.intellij.openapi.vcs.changes.*
@@ -199,6 +201,17 @@ abstract class BaseLineStatusTrackerManagerTest : LightPlatformTestCase() {
   }
 
 
+  fun runBatchFileChangeOperation(task: () -> Unit) {
+    BackgroundTaskUtil.syncPublisher(BatchFileChangeListener.TOPIC).batchChangeStarted(ourProject, "Update")
+    try {
+      task()
+    }
+    finally {
+      BackgroundTaskUtil.syncPublisher(BatchFileChangeListener.TOPIC).batchChangeCompleted(ourProject)
+    }
+  }
+
+
   protected fun createChangelist(listName: String) {
     assertDoesntContain(changeListsNames(), listName)
     clm.addChangeList(listName, null)

platform/vcs-tests/testSrc/com/intellij/openapi/vcs/LineStatusTrackerManagerTest.kt

@@ -332,4 +332,76 @@ class LineStatusTrackerManagerTest : BaseLineStatusTrackerManagerTest() {
       tracker.assertAffectedChangeLists("Default")
     }
   }
+
+  fun `test bulk refresh freezes tracker - inner operation`() {
+    val file = addLocalFile(FILE_1, "a_b_c_d_e2")
+    setBaseVersion(FILE_1, "a_b_c_d_e")
+    refreshCLM()
+
+    file.withOpenedEditor {
+      val tracker = file.tracker as PartialLocalLineStatusTracker
+      lstm.waitUntilBaseContentsLoaded()
+      assertTrue(tracker.isValid())
+
+      runBatchFileChangeOperation {
+        assertFalse(tracker.isValid())
+      }
+      assertTrue(tracker.isValid())
+    }
+    assertNull(file.tracker)
+  }
+
+  fun `test bulk refresh freezes tracker - outer operation`() {
+    val file = addLocalFile(FILE_1, "a_b_c_d_e2")
+    setBaseVersion(FILE_1, "a_b_c_d_e")
+    refreshCLM()
+
+    runBatchFileChangeOperation {
+      file.withOpenedEditor {
+        val tracker = file.tracker as PartialLocalLineStatusTracker
+        lstm.waitUntilBaseContentsLoaded()
+        assertFalse(tracker.isValid())
+      }
+    }
+    assertNull(file.tracker)
+  }
+
+  fun `test bulk refresh freezes tracker - interleaving operation`() {
+    val file = addLocalFile(FILE_1, "a_b_c_d_e2")
+    setBaseVersion(FILE_1, "a_b_c_d_e")
+    refreshCLM()
+
+    runBatchFileChangeOperation {
+      lstm.requestTrackerFor(file.document, this)
+      lstm.waitUntilBaseContentsLoaded()
+
+      val tracker = file.tracker as PartialLocalLineStatusTracker
+      assertFalse(tracker.isValid())
+    }
+    val tracker = file.tracker as PartialLocalLineStatusTracker
+    assertTrue(tracker.isValid())
+    lstm.releaseTrackerFor(file.document, this)
+    assertNull(file.tracker)
+  }
+
+  fun `test bulk refresh freezes tracker - multiple operations`() {
+    val file = addLocalFile(FILE_1, "a_b_c_d_e2")
+    setBaseVersion(FILE_1, "a_b_c_d_e")
+    refreshCLM()
+
+    file.withOpenedEditor {
+      val tracker = file.tracker as PartialLocalLineStatusTracker
+      lstm.waitUntilBaseContentsLoaded()
+      assertTrue(tracker.isValid())
+
+      runBatchFileChangeOperation {
+        runBatchFileChangeOperation {
+          assertFalse(tracker.isValid())
+        }
+        assertFalse(tracker.isValid())
+      }
+      assertTrue(tracker.isValid())
+    }
+    assertNull(file.tracker)
+  }
 }