From 4d8680e81c9dc93ac78d2b4d3a03264ecabfbf1b Mon Sep 17 00:00:00 2001 From: Aleksey Pivovarov Date: Fri, 27 Dec 2024 14:14:11 +0100 Subject: [PATCH] IJPL-162494 git: do not load git content from EditorHistoryManager Potemking progress during service initialization may deadlock during event pumping. GitOrigin-RevId: b405124f5fb7644d5dd9c5e10a2caaa935fc5665 --- .../src/git4idea/index/GitStageDiffUtil.kt | 2 +- .../GitStageLineStatusTrackerProvider.kt | 2 +- .../actions/GitStageShowVersionAction.kt | 38 +++++-- .../git4idea/index/vfs/GitIndexFileSystem.kt | 7 +- .../index/vfs/GitIndexFileSystemRefresher.kt | 99 ++++++++++--------- .../git4idea/index/vfs/GitIndexVirtualFile.kt | 37 ++++--- .../git4idea/index/GitStageTrackerTest.kt | 2 +- 7 files changed, 115 insertions(+), 72 deletions(-) diff --git a/plugins/git4idea/src/git4idea/index/GitStageDiffUtil.kt b/plugins/git4idea/src/git4idea/index/GitStageDiffUtil.kt index c5cd1c87e95e..85dce43d2eab 100644 --- a/plugins/git4idea/src/git4idea/index/GitStageDiffUtil.kt +++ b/plugins/git4idea/src/git4idea/index/GitStageDiffUtil.kt @@ -179,7 +179,7 @@ private fun headContentBytes(project: Project, root: VirtualFile, status: GitFil @Throws(VcsException::class) private fun stagedContentFile(project: Project, root: VirtualFile, status: GitFileStatus): VirtualFile { val filePath = status.path(ContentVersion.STAGED) - return GitIndexFileSystemRefresher.getInstance(project).getFile(root, filePath) + return GitIndexFileSystemRefresher.getInstance(project).createFile(root, filePath) ?: throw VcsException(GitBundle.message("stage.diff.staged.content.exception.message", status.path)) } diff --git a/plugins/git4idea/src/git4idea/index/GitStageLineStatusTrackerProvider.kt b/plugins/git4idea/src/git4idea/index/GitStageLineStatusTrackerProvider.kt index 449e2c2e116d..1b965933e23d 100644 --- a/plugins/git4idea/src/git4idea/index/GitStageLineStatusTrackerProvider.kt +++ b/plugins/git4idea/src/git4idea/index/GitStageLineStatusTrackerProvider.kt @@ -77,7 +77,7 @@ class GitStageLineStatusTrackerProvider : LineStatusTrackerContentLoader { val repository = GitRepositoryManager.getInstance(project).getRepositoryForFile(file) ?: return null val indexFileRefresher = GitIndexFileSystemRefresher.getInstance(project) - val indexFile = indexFileRefresher.getFile(repository.root, status.path(ContentVersion.STAGED)) ?: return null + val indexFile = indexFileRefresher.createFile(repository.root, status.path(ContentVersion.STAGED)) ?: return null val indexDocument = runReadAction { FileDocumentManager.getInstance().getDocument(indexFile) } ?: return null indexDocument.putUserData(LineStatusTrackerBase.SEPARATE_UNDO_STACK, Registry.`is`("git.stage.separate.undo.stack")) diff --git a/plugins/git4idea/src/git4idea/index/actions/GitStageShowVersionAction.kt b/plugins/git4idea/src/git4idea/index/actions/GitStageShowVersionAction.kt index 7b9c39c10025..97d1ddd351ed 100644 --- a/plugins/git4idea/src/git4idea/index/actions/GitStageShowVersionAction.kt +++ b/plugins/git4idea/src/git4idea/index/actions/GitStageShowVersionAction.kt @@ -4,6 +4,7 @@ package git4idea.index.actions import com.intellij.openapi.actionSystem.ActionUpdateThread import com.intellij.openapi.actionSystem.AnActionEvent import com.intellij.openapi.actionSystem.CommonDataKeys +import com.intellij.openapi.application.EDT import com.intellij.openapi.editor.Caret import com.intellij.openapi.editor.LogicalPosition import com.intellij.openapi.fileEditor.OpenFileDescriptor @@ -11,11 +12,18 @@ import com.intellij.openapi.project.DumbAwareAction import com.intellij.openapi.project.Project import com.intellij.openapi.vcs.impl.LineStatusTrackerManager import com.intellij.openapi.vfs.VirtualFile +import com.intellij.platform.ide.progress.withBackgroundProgress +import com.intellij.platform.util.coroutines.childScope import com.intellij.util.OpenSourceUtil +import git4idea.GitDisposable +import git4idea.i18n.GitBundle import git4idea.index.* import git4idea.index.vfs.GitIndexFileSystemRefresher import git4idea.index.vfs.GitIndexVirtualFile import git4idea.index.vfs.filePath +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.async +import kotlinx.coroutines.withContext abstract class GitStageShowVersionAction(private val showStaged: Boolean) : DumbAwareAction() { override fun getActionUpdateThread(): ActionUpdateThread { @@ -39,21 +47,33 @@ abstract class GitStageShowVersionAction(private val showStaged: Boolean) : Dumb val project = e.project ?: return val sourceFile = e.getData(CommonDataKeys.VIRTUAL_FILE) ?: return val root = getRoot(project, sourceFile) ?: return - val targetFile = if (showStaged) { - GitIndexFileSystemRefresher.getInstance(project).getFile(root, sourceFile.filePath()) + val caret = e.getData(CommonDataKeys.CARET) + + if (showStaged) { + GitDisposable.getInstance(project).coroutineScope.childScope("Show staged file").async { + val filePath = sourceFile.filePath() + withBackgroundProgress(project, GitBundle.message("stage.vfs.read.process", filePath.name), true) { + val targetFile = GitIndexFileSystemRefresher.getInstance(project).createFile(root, filePath) ?: return@withBackgroundProgress + withContext(Dispatchers.EDT) { + navigateToFile(project, targetFile, sourceFile, caret) + } + } + } } else { - sourceFile.filePath().virtualFile - } ?: return + val targetFile = sourceFile.filePath().virtualFile ?: return + navigateToFile(project, targetFile, sourceFile, caret) + } + } - val caret = e.getData(CommonDataKeys.CARET) + private fun navigateToFile(project: Project, targetFile: VirtualFile, sourceFile: VirtualFile, caret: Caret?) { if (caret == null) { OpenSourceUtil.navigate(OpenFileDescriptor(project, targetFile)) - return } - - val targetPosition = getTargetPosition(project, sourceFile, targetFile, caret) - OpenSourceUtil.navigate(OpenFileDescriptor(project, targetFile, targetPosition.line, targetPosition.column)) + else { + val targetPosition = getTargetPosition(project, sourceFile, targetFile, caret) + OpenSourceUtil.navigate(OpenFileDescriptor(project, targetFile, targetPosition.line, targetPosition.column)) + } } private fun getTargetPosition(project: Project, sourceFile: VirtualFile, targetFile: VirtualFile, caret: Caret): LogicalPosition { diff --git a/plugins/git4idea/src/git4idea/index/vfs/GitIndexFileSystem.kt b/plugins/git4idea/src/git4idea/index/vfs/GitIndexFileSystem.kt index f8bb59e73a69..3ca52140813b 100644 --- a/plugins/git4idea/src/git4idea/index/vfs/GitIndexFileSystem.kt +++ b/plugins/git4idea/src/git4idea/index/vfs/GitIndexFileSystem.kt @@ -15,9 +15,12 @@ class GitIndexFileSystem : VirtualFileSystem(), NonPhysicalFileSystem { override fun isReadOnly(): Boolean = false override fun findFileByPath(path: String): VirtualFile? { - val (project, virtualFile, filePath) = GitIndexVirtualFile.decode(path) ?: return null + val (project, root, filePath) = GitIndexVirtualFile.decode(path) ?: return null - return GitIndexFileSystemRefresher.getInstance(project).getFile(virtualFile, filePath) + // we do not check actual content here + // the returned file is valid, but may stop being such after the first read access + // this is a tradeoff with making EditorHistoryManager load every file that was ever loaded + return GitIndexFileSystemRefresher.getInstance(project).findFile(root, filePath) } override fun refreshAndFindFileByPath(path: String): VirtualFile? = findFileByPath(path) diff --git a/plugins/git4idea/src/git4idea/index/vfs/GitIndexFileSystemRefresher.kt b/plugins/git4idea/src/git4idea/index/vfs/GitIndexFileSystemRefresher.kt index 725e25ba17fe..6aa41f749688 100644 --- a/plugins/git4idea/src/git4idea/index/vfs/GitIndexFileSystemRefresher.kt +++ b/plugins/git4idea/src/git4idea/index/vfs/GitIndexFileSystemRefresher.kt @@ -15,7 +15,6 @@ import com.intellij.openapi.editor.Document import com.intellij.openapi.fileEditor.FileDocumentManager import com.intellij.openapi.fileTypes.FileTypeRegistry import com.intellij.openapi.fileTypes.StdFileTypes -import com.intellij.openapi.progress.ProcessCanceledException import com.intellij.openapi.progress.ProgressManager import com.intellij.openapi.progress.util.BackgroundTaskUtil import com.intellij.openapi.progress.util.PotemkinProgress @@ -24,7 +23,6 @@ import com.intellij.openapi.project.ProjectCloseListener import com.intellij.openapi.util.Disposer import com.intellij.openapi.util.NlsContexts import com.intellij.openapi.util.Ref -import com.intellij.openapi.util.ThrowableComputable import com.intellij.openapi.vcs.FilePath import com.intellij.openapi.vcs.VcsException import com.intellij.openapi.vfs.VfsUtil @@ -36,11 +34,11 @@ import com.intellij.openapi.vfs.newvfs.events.VFileContentChangeEvent import com.intellij.openapi.vfs.newvfs.events.VFileEvent.REFRESH_REQUESTOR import com.intellij.util.LocalTimeCounter import com.intellij.util.concurrency.AppExecutorUtil +import com.intellij.util.concurrency.annotations.RequiresBackgroundThread import com.intellij.util.messages.MessageBusConnection import com.intellij.vcs.log.Hash import com.intellij.vcs.log.impl.HashImpl import com.intellij.vcsUtil.VcsFileUtil -import com.intellij.vcsUtil.VcsUtil import git4idea.commands.Git import git4idea.commands.GitCommand import git4idea.commands.GitLineHandler @@ -53,7 +51,6 @@ import org.jetbrains.annotations.NonNls import java.io.ByteArrayInputStream import java.io.IOException import java.util.concurrent.TimeUnit -import kotlin.Throws @Service(Service.Level.PROJECT) class GitIndexFileSystemRefresher(private val project: Project) : Disposable { @@ -90,44 +87,53 @@ class GitIndexFileSystemRefresher(private val project: Project) : Disposable { connection.subscribe(EncodingManagerListener.ENCODING_MANAGER_CHANGES, MyEncodingManagerListener()) } - fun getFile(root: VirtualFile, filePath: FilePath): GitIndexVirtualFile? { - try { - return cache.get(Key(root, filePath)) - } - catch (e: Exception) { - val cause = e.cause - if (cause is ProcessCanceledException) { - throw cause - } - throw e - } + fun findFile( + root: VirtualFile, filePath: FilePath, + ): GitIndexVirtualFile? { + val indexFile = cache.get(Key(root, filePath)) + + if (indexFile.isValid()) return indexFile + + return null } - private fun createIndexVirtualFile(root: VirtualFile, filePath: FilePath): GitIndexVirtualFile? { - if (isShutDown) return null + @RequiresBackgroundThread + fun createFile( + root: VirtualFile, filePath: FilePath, + ): GitIndexVirtualFile? { + val indexFile = cache.get(Key(root, filePath)) + + if (indexFile.data != null) return indexFile val stagedFile = readMetadataFromGit(root, filePath) ?: return null val length = readLengthFromGit(root, stagedFile.blobHash) - val indexFile = GitIndexVirtualFile(project, root, filePath, stagedFile.hash(), length, stagedFile.isExecutable) - OutsidersPsiFileSupport.markFile(indexFile, filePath) + + indexFile.setInitialData(stagedFile.hash(), length, stagedFile.isExecutable) + return indexFile } private fun createIndexVirtualFile(key: Key): GitIndexVirtualFile? { - if (!ApplicationManager.getApplication().isDispatchThread) return createIndexVirtualFile(key.root, key.filePath) + if (isShutDown) return null - return ProgressManager.getInstance().runProcessWithProgressSynchronously(ThrowableComputable { - createIndexVirtualFile(key.root, key.filePath) - }, GitBundle.message("stage.vfs.read.process", key.filePath.name), false, project) + val indexFile = GitIndexVirtualFile(project, key.root, key.filePath) + OutsidersPsiFileSupport.markFile(indexFile, key.filePath) + return indexFile } fun refresh(condition: (GitIndexVirtualFile) -> Boolean) { - val filesToRefresh = cache.asMap().values.filter(condition) + val filesToRefresh = cache.asMap().values + .filter { it.data != null } + .filter(condition) if (filesToRefresh.isEmpty()) return - refresh(filesToRefresh) + refreshImpl(filesToRefresh) } - private fun refresh(filesToRefresh: List) { + fun initialRefresh(filesToRefresh: List) { + refreshImpl(filesToRefresh) + } + + private fun refreshImpl(filesToRefresh: List) { if (isShutDown) return LOG.debug("Starting async refresh for ${filesToRefresh.joinToString { it.path }}") @@ -151,19 +157,21 @@ class GitIndexFileSystemRefresher(private val project: Project) : Disposable { } private fun readFromGit(file: GitIndexVirtualFile): IndexFileData? { - val (oldHash, oldModificationStamp) = runReadAction { Pair(file.hash, file.modificationStamp) } - return readFromGit(file, oldHash, oldModificationStamp) + val (oldData, oldModificationStamp) = runReadAction { Pair(file.data, file.modificationStamp) } + return readFromGit(file, oldData, oldModificationStamp) } - private fun readFromGit(file: GitIndexVirtualFile, - oldHash: Hash?, - oldModificationStamp: Long): IndexFileData? { + private fun readFromGit( + file: GitIndexVirtualFile, + oldData: GitIndexVirtualFile.CachedData?, + oldModificationStamp: Long, + ): IndexFileData? { val stagedFile = readMetadataFromGit(file.root, file.filePath) val newHash = stagedFile?.hash() - if (oldHash != newHash) { + if (oldData == null || oldData.hash != newHash) { val newLength = if (stagedFile != null) readLengthFromGit(file.root, stagedFile.blobHash) else 0 LOG.debug("Preparing refresh for $file") - return IndexFileData(file, oldHash, newHash, file.length, newLength, stagedFile?.isExecutable ?: false, oldModificationStamp) + return IndexFileData(file, oldData, newHash, file.length, newLength, stagedFile?.isExecutable ?: false, oldModificationStamp) } return null } @@ -181,11 +189,11 @@ class GitIndexFileSystemRefresher(private val project: Project) : Disposable { val event = VFileContentChangeEvent(requestor, file, file.modificationStamp, newModStamp) ApplicationManager.getApplication().messageBus.syncPublisher(VirtualFileManager.VFS_CHANGES).before(listOf(event)) - val oldHash = file.hash + val oldData = file.data val oldModificationStamp = file.modificationStamp val applyChanges = computeUnderPotemkinProgress(project, GitBundle.message("stage.vfs.write.process", file.name)) { - val indexFileData = readFromGit(file, oldHash, oldModificationStamp) + val indexFileData = readFromGit(file, oldData, oldModificationStamp) if (indexFileData != null) { LOG.info("Detected memory-disk conflict in $file") return@computeUnderPotemkinProgress { @@ -263,7 +271,7 @@ class GitIndexFileSystemRefresher(private val project: Project) : Disposable { @JvmStatic fun refreshVirtualFiles(project: Project, paths: Collection) { - refreshFilePaths(project, paths.map(VcsUtil::getFilePath)) + refreshFilePaths(project, paths.map { it.filePath() }) } @JvmStatic @@ -292,16 +300,19 @@ class GitIndexFileSystemRefresher(private val project: Project) : Disposable { } } - private inner class IndexFileData(private val file: GitIndexVirtualFile, - private val oldHash: Hash?, - private val newHash: Hash?, - oldLength: Long, - private val newLength: Long, - private val newExecutable: Boolean, - oldModificationStamp: Long) { + private inner class IndexFileData( + private val file: GitIndexVirtualFile, + private val oldData: GitIndexVirtualFile.CachedData?, + private val newHash: Hash?, + oldLength: Long, + private val newLength: Long, + private val newExecutable: Boolean, + oldModificationStamp: Long, + ) { val event = VFileContentChangeEvent(REFRESH_REQUESTOR, file, oldModificationStamp, -1, 0, 0, oldLength, newLength) - fun isOutdated() = file.hash != oldHash + fun isOutdated() = file.data != null && + file.data?.hash != oldData?.hash fun apply() { LOG.debug("Refreshing $file") diff --git a/plugins/git4idea/src/git4idea/index/vfs/GitIndexVirtualFile.kt b/plugins/git4idea/src/git4idea/index/vfs/GitIndexVirtualFile.kt index 8ec4f29682cc..7dad1622f759 100644 --- a/plugins/git4idea/src/git4idea/index/vfs/GitIndexVirtualFile.kt +++ b/plugins/git4idea/src/git4idea/index/vfs/GitIndexVirtualFile.kt @@ -24,18 +24,19 @@ import git4idea.i18n.GitBundle import org.jetbrains.annotations.NonNls import java.io.* import java.util.* -import kotlin.Throws -class GitIndexVirtualFile(private val project: Project, - val root: VirtualFile, - val filePath: FilePath, - @Volatile internal var hash: Hash?, - @Volatile internal var length: Long, - @Volatile internal var isExecutable: Boolean) : VirtualFile(), VirtualFilePathWrapper { +class GitIndexVirtualFile( + private val project: Project, + val root: VirtualFile, + val filePath: FilePath, +) : VirtualFile(), VirtualFilePathWrapper { init { putUserData(FileDocumentManagerBase.TRACK_NON_PHYSICAL, true) } + @Volatile + internal var data: CachedData? = null // null: the file was not loaded yet + @Volatile private var modificationStamp = LocalTimeCounter.currentTime() @@ -44,13 +45,14 @@ class GitIndexVirtualFile(private val project: Project, override fun getChildren(): Array = EMPTY_ARRAY override fun isWritable(): Boolean = true override fun isDirectory(): Boolean = false - override fun isValid(): Boolean = hash != null && !project.isDisposed + override fun isValid(): Boolean = !project.isDisposed && (data == null || data?.hash != null) override fun getName(): String = filePath.name override fun getPresentableName(): String = GitBundle.message("stage.vfs.presentable.file.name", filePath.name) override fun getPath(): String = encode(project, root, filePath) override fun getPresentablePath(): String = filePath.path override fun enforcePresentableName(): Boolean = true - override fun getLength(): Long = length + internal val isExecutable: Boolean get() = data?.isExecutable ?: false + override fun getLength(): Long = data?.length ?: 0 override fun getTimeStamp(): Long = 0 override fun getModificationStamp(): Long = modificationStamp override fun getFileType(): FileType = filePath.virtualFile?.fileType ?: super.getFileType() @@ -58,17 +60,18 @@ class GitIndexVirtualFile(private val project: Project, LOG.error("Refreshing index files is not supported (called for $this). Use GitIndexFileSystemRefresher to refresh.") } + internal fun setInitialData(newHash: Hash?, newLength: Long, newExecutable: Boolean) { + data = CachedData(newHash, newLength, newExecutable) + } + @RequiresWriteLock internal fun setDataFromRefresh(newHash: Hash?, newLength: Long, newExecutable: Boolean) { - hash = newHash - length = newLength - isExecutable = newExecutable + data = CachedData(newHash, newLength, newExecutable) } @RequiresWriteLock internal fun setDataFromWrite(newHash: Hash, newLength: Long, newModificationStamp: Long) { - hash = newHash - length = newLength + data = CachedData(newHash, newLength, isExecutable) modificationStamp = newModificationStamp } @@ -91,6 +94,10 @@ class GitIndexVirtualFile(private val project: Project, @Throws(IOException::class) override fun contentsToByteArray(): ByteArray { + if (data == null) { + GitIndexFileSystemRefresher.getInstance(project).initialRefresh(listOf(this)) + } + try { if (ApplicationManager.getApplication().isDispatchThread) { return ProgressManager.getInstance().runProcessWithProgressSynchronously(ThrowableComputable { @@ -153,6 +160,8 @@ class GitIndexVirtualFile(private val project: Project, return path.substringAfterLast(SEPARATOR).replace('/', File.separatorChar) } } + + internal data class CachedData(val hash: Hash?, val length: Long, val isExecutable: Boolean) } internal fun VirtualFile.filePath(): FilePath { diff --git a/plugins/git4idea/tests/git4idea/index/GitStageTrackerTest.kt b/plugins/git4idea/tests/git4idea/index/GitStageTrackerTest.kt index 7540c9992a75..fe4395337d4d 100644 --- a/plugins/git4idea/tests/git4idea/index/GitStageTrackerTest.kt +++ b/plugins/git4idea/tests/git4idea/index/GitStageTrackerTest.kt @@ -102,7 +102,7 @@ class GitStageTrackerTest : GitSingleRepoTest() { assertTrue(trackerState().isEmpty()) val file = projectRoot.findChild(fileName)!! - val indexFile = project.service().getFile(projectRoot, VcsUtil.getFilePath(file))!! + val indexFile = project.service().createFile(projectRoot, VcsUtil.getFilePath(file))!! val document = runReadAction { FileDocumentManager.getInstance().getDocument(indexFile)!! } runWithTrackerUpdate("setText") {