[Workspace Model] [IDEA-324533] Clean up usages of workspace model listener that may lead to control-flow issues

If we create a subscription between before and after events, this may cause issues in code logic. Also, such issues will be really hard to debug.
I've reviewed the listeners of the workspace model in order to get rid of such problem.
See IDEA-324532 for details.

GitOrigin-RevId: c5b8c12691fa930d3f5f85af32052e0a22984d73
This commit is contained in:
Alex Plate
2023-07-06 18:01:14 +03:00
committed by intellij-monorepo-bot
parent c73162424c
commit fd353843d1
6 changed files with 59 additions and 37 deletions

View File

@@ -25,6 +25,12 @@ import com.intellij.openapi.vfs.AsyncFileListener;
import com.intellij.openapi.vfs.VirtualFile;
import com.intellij.openapi.vfs.VirtualFileManager;
import com.intellij.openapi.vfs.newvfs.events.*;
import com.intellij.platform.backend.workspace.WorkspaceModelChangeListener;
import com.intellij.platform.backend.workspace.WorkspaceModelTopics;
import com.intellij.platform.workspace.jps.entities.ContentRootEntity;
import com.intellij.platform.workspace.jps.entities.ModuleEntity;
import com.intellij.platform.workspace.storage.EntityChange;
import com.intellij.platform.workspace.storage.VersionedStorageChange;
import com.intellij.psi.PsiDocumentManager;
import com.intellij.psi.PsiFile;
import com.intellij.psi.search.GlobalSearchScope;
@@ -34,13 +40,7 @@ import com.intellij.psi.util.PsiModificationTracker;
import com.intellij.util.SmartList;
import com.intellij.util.containers.ContainerUtil;
import com.intellij.util.messages.MessageBusConnection;
import com.intellij.platform.backend.workspace.WorkspaceModelChangeListener;
import com.intellij.platform.backend.workspace.WorkspaceModelTopics;
import com.intellij.workspaceModel.ide.impl.legacyBridge.module.ModuleEntityUtils;
import com.intellij.platform.workspace.storage.EntityChange;
import com.intellij.platform.workspace.storage.VersionedStorageChange;
import com.intellij.platform.workspace.jps.entities.ContentRootEntity;
import com.intellij.platform.workspace.jps.entities.ModuleEntity;
import org.jetbrains.annotations.Contract;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
@@ -96,22 +96,6 @@ public final class DirtyScopeHolder extends UserDataHolderBase implements AsyncF
compilationAffectedModulesSubscription.accept(connect, myCompilationAffectedModules);
connect.subscribe(WorkspaceModelTopics.CHANGED, new WorkspaceModelChangeListener() {
@Override
public void beforeChanged(@NotNull VersionedStorageChange event) {
for (EntityChange<ModuleEntity> change : event.getChanges(ModuleEntity.class)) {
ModuleEntity oldEntity = change.getOldEntity();
if (oldEntity != null) {
addToDirtyModules(ModuleEntityUtils.findModule(oldEntity, event.getStorageBefore()));
}
}
for (EntityChange<ContentRootEntity> change : event.getChanges(ContentRootEntity.class)) {
ContentRootEntity oldEntity = change.getOldEntity();
if (oldEntity != null) {
addToDirtyModules(ModuleEntityUtils.findModule(oldEntity.getModule(), event.getStorageBefore()));
}
}
}
@Override
public void changed(@NotNull VersionedStorageChange event) {
for (EntityChange<ModuleEntity> change : event.getChanges(ModuleEntity.class)) {
@@ -119,12 +103,20 @@ public final class DirtyScopeHolder extends UserDataHolderBase implements AsyncF
if (newEntity != null) {
addToDirtyModules(ModuleEntityUtils.findModule(newEntity, event.getStorageAfter()));
}
ModuleEntity oldEntity = change.getOldEntity();
if (oldEntity != null) {
addToDirtyModules(ModuleEntityUtils.findModule(oldEntity, event.getStorageBefore()));
}
}
for (EntityChange<ContentRootEntity> change : event.getChanges(ContentRootEntity.class)) {
ContentRootEntity newEntity = change.getNewEntity();
if (newEntity != null) {
addToDirtyModules(ModuleEntityUtils.findModule(newEntity.getModule(), event.getStorageAfter()));
}
ContentRootEntity oldEntity = change.getOldEntity();
if (oldEntity != null) {
addToDirtyModules(ModuleEntityUtils.findModule(oldEntity.getModule(), event.getStorageBefore()));
}
}
clearDisposedModules();
}

View File

@@ -58,7 +58,15 @@ internal class GlobalChangedRepositoryLibrarySynchronizer(private val queue: Lib
internal class ChangedRepositoryLibrarySynchronizer(private val project: Project,
private val queue: LibrarySynchronizationQueue) : WorkspaceModelChangeListener {
/**
* This is a flag indicating that the [beforeChanged] method was called. Due to the fact that we subscribe using the code, this
* may lead to IDEA-324532.
* With this flag we skip the "after" event if the before event wasn't called.
*/
private var beforeCalled = false
override fun beforeChanged(event: VersionedStorageChange) {
beforeCalled = true
for (change in event.getChanges(LibraryEntity::class.java)) {
val removed = change as? EntityChange.Removed ?: continue
val library = findLibrary(removed.entity.symbolicId, event.storageBefore)
@@ -82,6 +90,8 @@ internal class ChangedRepositoryLibrarySynchronizer(private val project: Project
}
override fun changed(event: VersionedStorageChange) {
if (!beforeCalled) return
beforeCalled = false
var libraryReloadRequested = false
for (change in event.getChanges(LibraryEntity::class.java)) {

View File

@@ -68,21 +68,19 @@ internal class FacetEntityChangeListener(private val project: Project, coroutine
initializeFacetBridgeTimeMs.addElapsedTimeMs(start)
}
init {
if (!project.isDefault) {
project.messageBus.connect(coroutineScope).subscribe(WorkspaceModelTopics.CHANGED, object : WorkspaceModelChangeListener {
override fun beforeChanged(event: VersionedStorageChange) {
WorkspaceFacetContributor.EP_NAME.extensions.forEach { facetBridgeContributor ->
processBeforeChangeEvents(event, facetBridgeContributor)
}
}
class WorkspaceModelListener(project: Project) : WorkspaceModelChangeListener {
private val facetEntityChangeListener = getInstance(project)
override fun changed(event: VersionedStorageChange) {
WorkspaceFacetContributor.EP_NAME.extensions.forEach { facetBridgeContributor ->
processChangeEvents(event, facetBridgeContributor)
}
}
})
override fun beforeChanged(event: VersionedStorageChange) {
WorkspaceFacetContributor.EP_NAME.extensions.forEach { facetBridgeContributor ->
facetEntityChangeListener.processBeforeChangeEvents(event, facetBridgeContributor)
}
}
override fun changed(event: VersionedStorageChange) {
WorkspaceFacetContributor.EP_NAME.extensions.forEach { facetBridgeContributor ->
facetEntityChangeListener.processChangeEvents(event, facetBridgeContributor)
}
}
}

View File

@@ -1749,5 +1749,7 @@
topic="com.intellij.codeInsight.lookup.LookupManagerListener"/>
<listener class="com.intellij.psi.impl.file.impl.PsiVFSListener$MyModuleRootListener"
topic="com.intellij.openapi.roots.ModuleRootListener"/>
<listener class="com.intellij.workspaceModel.ide.impl.legacyBridge.facet.FacetEntityChangeListener$WorkspaceModelListener"
topic="com.intellij.platform.backend.workspace.WorkspaceModelChangeListener"/>
</projectListeners>
</idea-plugin>

View File

@@ -68,7 +68,15 @@ class ProjectLibraryTableBridgeImpl(
init {
project.messageBus.connect(this).subscribe(WorkspaceModelTopics.CHANGED, object : WorkspaceModelChangeListener {
/**
* This is a flag indicating that the [beforeChanged] method was called. Due to the fact that we subscribe using the code, this
* may lead to IDEA-324532.
* With this flag we skip the "after" event if the before event wasn't called.
*/
private var beforeCalled = false
override fun beforeChanged(event: VersionedStorageChange) {
beforeCalled = true
val libraryChanges = event.getChanges(LibraryEntity::class.java)
val removeChanges = libraryChanges.filterProjectLibraryChanges().filterIsInstance<EntityChange.Removed<LibraryEntity>>()
if (removeChanges.isEmpty()) return
@@ -83,6 +91,8 @@ class ProjectLibraryTableBridgeImpl(
}
override fun changed(event: VersionedStorageChange) {
if (!beforeCalled) return
beforeCalled = false
val changes = event.getChanges(LibraryEntity::class.java).filterProjectLibraryChanges()
.orderToRemoveReplaceAdd() // Since the listener is not deprecated, it will be better to keep the order of events as remove -> replace -> add
if (changes.isEmpty()) return

View File

@@ -33,9 +33,17 @@ internal class LegacyProjectModelListenersBridge(
private val moduleModificationTracker: SimpleModificationTracker,
private val moduleRootListenerBridge: ModuleRootListenerBridge
) : WorkspaceModelChangeListener {
/**
* This is a flag indicating that the [beforeChanged] method was called. Due to the fact that we subscribe using the code, this
* may lead to IDEA-324532.
* With this flag we skip the "after" event if the before event wasn't called.
*/
private var beforeCalled = false
override fun beforeChanged(event: VersionedStorageChange) {
LOG.trace { "Get before changed event" }
beforeCalled = true
moduleRootListenerBridge.fireBeforeRootsChanged(project, event)
val moduleMap = event.storageBefore.moduleMap
for (change in event.getChanges(ModuleEntity::class.java)) {
@@ -50,6 +58,8 @@ internal class LegacyProjectModelListenersBridge(
}
override fun changed(event: VersionedStorageChange) {
if (!beforeCalled) return
beforeCalled = false
LOG.trace { "Get changed event" }
val moduleLibraryChanges = event.getChanges(LibraryEntity::class.java).filterModuleLibraryChanges()
val changes = event.getChanges(ModuleEntity::class.java)