From df49bfbecde428cd235142f211fc7d687a07f41e Mon Sep 17 00:00:00 2001 From: Alexey Kudravtsev Date: Tue, 20 Nov 2018 15:34:22 +0300 Subject: [PATCH] fix many leaking threads in tests --- .../RepositoryLibrarySynchronizer.java | 20 +++++----- .../daemon/impl/DaemonCodeAnalyzerImpl.java | 37 +++++++++++++++---- .../GeneratedSourceFileChangeTrackerImpl.java | 15 ++++++-- .../testFramework/LightPlatformTestCase.java | 2 + .../testFramework/PlatformTestCase.java | 2 + .../testFramework/UsefulTestCase.java | 8 ++++ .../fixtures/CodeInsightFixtureTestCase.java | 4 ++ .../fixtures/impl/BaseFixture.java | 9 +++++ 8 files changed, 76 insertions(+), 21 deletions(-) diff --git a/java/idea-ui/src/com/intellij/jarRepository/RepositoryLibrarySynchronizer.java b/java/idea-ui/src/com/intellij/jarRepository/RepositoryLibrarySynchronizer.java index 79582953d67c..48031992c11b 100644 --- a/java/idea-ui/src/com/intellij/jarRepository/RepositoryLibrarySynchronizer.java +++ b/java/idea-ui/src/com/intellij/jarRepository/RepositoryLibrarySynchronizer.java @@ -164,18 +164,18 @@ public class RepositoryLibrarySynchronizer implements StartupActivity, DumbAware }, project.getDisposed()); }; - project.getMessageBus().connect().subscribe(ProjectTopics.PROJECT_ROOTS, new ModuleRootListener() { - private final Alarm myAlarm = new Alarm(Alarm.ThreadToUse.POOLED_THREAD, project); - @Override - public void rootsChanged(@NotNull final ModuleRootEvent event) { - if (!myAlarm.isDisposed() && event.getSource() instanceof Project) { - myAlarm.cancelAllRequests(); - myAlarm.addRequest(syncTask, 300L); - } - } - }); if (!ApplicationManager.getApplication().isUnitTestMode()) { + project.getMessageBus().connect().subscribe(ProjectTopics.PROJECT_ROOTS, new ModuleRootListener() { + private final Alarm myAlarm = new Alarm(Alarm.ThreadToUse.POOLED_THREAD, project); + @Override + public void rootsChanged(@NotNull final ModuleRootEvent event) { + if (!myAlarm.isDisposed() && event.getSource() instanceof Project) { + myAlarm.cancelAllRequests(); + myAlarm.addRequest(syncTask, 300L); + } + } + }); ApplicationManager.getApplication().executeOnPooledThread(() -> { removeDuplicatedUrlsFromRepositoryLibraries(project); syncTask.run(); diff --git a/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/DaemonCodeAnalyzerImpl.java b/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/DaemonCodeAnalyzerImpl.java index 2ffbbea1780e..9ad28190b9c1 100644 --- a/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/DaemonCodeAnalyzerImpl.java +++ b/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/DaemonCodeAnalyzerImpl.java @@ -323,15 +323,14 @@ public class DaemonCodeAnalyzerImpl extends DaemonCodeAnalyzerEx implements Pers FileStatusMap fileStatusMap = getFileStatusMap(); Map map = new HashMap<>(); + + try { + waitForAllEditorsFinallyLoaded(project, 10, TimeUnit.SECONDS); + } + catch (TimeoutException e) { + throw new RuntimeException("editors have not completed loading in 10 seconds"); + } for (TextEditor textEditor : textEditors) { - if (textEditor instanceof TextEditorImpl) { - try { - ((TextEditorImpl)textEditor).waitForLoaded(10, TimeUnit.SECONDS); - } - catch (TimeoutException e) { - throw new RuntimeException(textEditor + " has not completed loading in 10 seconds"); - } - } TextEditorBackgroundHighlighter highlighter = (TextEditorBackgroundHighlighter)textEditor.getBackgroundHighlighter(); if (highlighter == null) { Editor editor = textEditor.getEditor(); @@ -392,6 +391,28 @@ public class DaemonCodeAnalyzerImpl extends DaemonCodeAnalyzerEx implements Pers } } + @TestOnly + public static void waitForAllEditorsFinallyLoaded(@NotNull Project project, long timeout, @NotNull TimeUnit unit) throws TimeoutException { + ApplicationManager.getApplication().assertIsDispatchThread(); + long deadline = unit.toMillis(timeout) + System.currentTimeMillis(); + W: + while (true) { + UIUtil.dispatchAllInvocationEvents(); + if (System.currentTimeMillis() > deadline) throw new TimeoutException(); + for (FileEditor editor : FileEditorManager.getInstance(project).getAllEditors()) { + if (editor instanceof TextEditorImpl) { + try { + ((TextEditorImpl)editor).waitForLoaded(1, TimeUnit.MILLISECONDS); + } + catch (TimeoutException ignored) { + continue W; + } + } + } + break; + } + } + @TestOnly private boolean waitInOtherThread(int millis, boolean canChangeDocument) throws Throwable { Disposable disposable = Disposer.newDisposable(); diff --git a/platform/lang-impl/src/com/intellij/ide/GeneratedSourceFileChangeTrackerImpl.java b/platform/lang-impl/src/com/intellij/ide/GeneratedSourceFileChangeTrackerImpl.java index 6e4b5f6e4f8d..f9a2df59db72 100644 --- a/platform/lang-impl/src/com/intellij/ide/GeneratedSourceFileChangeTrackerImpl.java +++ b/platform/lang-impl/src/com/intellij/ide/GeneratedSourceFileChangeTrackerImpl.java @@ -3,6 +3,8 @@ package com.intellij.ide; import com.intellij.AppTopics; import com.intellij.ProjectTopics; +import com.intellij.openapi.Disposable; +import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.application.ReadAction; import com.intellij.openapi.components.ProjectComponent; import com.intellij.openapi.editor.Document; @@ -32,7 +34,7 @@ import java.util.concurrent.TimeUnit; /** * @author nik */ -public class GeneratedSourceFileChangeTrackerImpl extends GeneratedSourceFileChangeTracker implements ProjectComponent { +public class GeneratedSourceFileChangeTrackerImpl extends GeneratedSourceFileChangeTracker implements ProjectComponent, Disposable { private final Project myProject; private final FileDocumentManager myDocumentManager; private final EditorNotifications myEditorNotifications; @@ -44,12 +46,19 @@ public class GeneratedSourceFileChangeTrackerImpl extends GeneratedSourceFileCha myProject = project; myDocumentManager = documentManager; myEditorNotifications = editorNotifications; - myCheckingQueue = new SingleAlarm(this::checkFiles, 500, Alarm.ThreadToUse.POOLED_THREAD, project); + myCheckingQueue = new SingleAlarm(this::checkFiles, 500, Alarm.ThreadToUse.POOLED_THREAD, this); + } + + @Override + public void dispose() { } @TestOnly public void waitForAlarm() throws Exception { - myCheckingQueue.waitForAllExecuted(1, TimeUnit.SECONDS); + if (ApplicationManager.getApplication().isWriteAccessAllowed()) { + throw new IllegalStateException("Must not wait for the alarm under write action"); + } + myCheckingQueue.waitForAllExecuted(10, TimeUnit.SECONDS); } @Override diff --git a/platform/testFramework/src/com/intellij/testFramework/LightPlatformTestCase.java b/platform/testFramework/src/com/intellij/testFramework/LightPlatformTestCase.java index bf111d50d0c3..22144d772b77 100644 --- a/platform/testFramework/src/com/intellij/testFramework/LightPlatformTestCase.java +++ b/platform/testFramework/src/com/intellij/testFramework/LightPlatformTestCase.java @@ -5,6 +5,7 @@ import com.intellij.ProjectTopics; import com.intellij.ReviseWhenPortedToJDK; import com.intellij.application.options.CodeStyle; import com.intellij.codeInsight.completion.CompletionProgressIndicator; +import com.intellij.codeInsight.daemon.impl.DaemonCodeAnalyzerImpl; import com.intellij.codeInsight.hint.HintManager; import com.intellij.codeInsight.hint.HintManagerImpl; import com.intellij.codeInsight.lookup.LookupManager; @@ -386,6 +387,7 @@ public abstract class LightPlatformTestCase extends UsefulTestCase implements Da // don't use method references here to make stack trace reading easier //noinspection Convert2MethodRef new RunAll( + () -> DaemonCodeAnalyzerImpl.waitForAllEditorsFinallyLoaded(project, 10, TimeUnit.SECONDS), () -> CodeStyle.dropTemporarySettings(project), () -> myCodeStyleSettingsTracker.checkForSettingsDamage(), () -> doTearDown(project, ourApplication), diff --git a/platform/testFramework/src/com/intellij/testFramework/PlatformTestCase.java b/platform/testFramework/src/com/intellij/testFramework/PlatformTestCase.java index b224861247c2..62174daa9bea 100644 --- a/platform/testFramework/src/com/intellij/testFramework/PlatformTestCase.java +++ b/platform/testFramework/src/com/intellij/testFramework/PlatformTestCase.java @@ -3,6 +3,7 @@ package com.intellij.testFramework; import com.intellij.application.options.CodeStyle; import com.intellij.codeInsight.AutoPopupController; +import com.intellij.codeInsight.daemon.impl.DaemonCodeAnalyzerImpl; import com.intellij.ide.highlighter.ModuleFileType; import com.intellij.ide.highlighter.ProjectFileType; import com.intellij.ide.startup.impl.StartupManagerImpl; @@ -500,6 +501,7 @@ public abstract class PlatformTestCase extends UsefulTestCase implements DataPro protected void tearDown() throws Exception { Project project = myProject; if (project != null && !project.isDisposed()) { + DaemonCodeAnalyzerImpl.waitForAllEditorsFinallyLoaded(project, 10, TimeUnit.SECONDS); AutoPopupController.getInstance(project).cancelAllRequests(); // clear "show param info" delayed requests leaking project } // don't use method references here to make stack trace reading easier diff --git a/platform/testFramework/src/com/intellij/testFramework/UsefulTestCase.java b/platform/testFramework/src/com/intellij/testFramework/UsefulTestCase.java index 65c32e67f6ca..61aa5459128a 100644 --- a/platform/testFramework/src/com/intellij/testFramework/UsefulTestCase.java +++ b/platform/testFramework/src/com/intellij/testFramework/UsefulTestCase.java @@ -5,6 +5,7 @@ import com.intellij.codeInsight.CodeInsightSettings; import com.intellij.concurrency.IdeaForkJoinWorkerThreadFactory; import com.intellij.diagnostic.PerformanceWatcher; import com.intellij.openapi.Disposable; +import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.application.PathManager; import com.intellij.openapi.application.impl.ApplicationInfoImpl; import com.intellij.openapi.command.impl.StartMarkAction; @@ -30,6 +31,8 @@ import com.intellij.util.containers.ContainerUtil; import com.intellij.util.containers.PeekableIterator; import com.intellij.util.containers.PeekableIteratorWrapper; import com.intellij.util.containers.hash.HashMap; +import com.intellij.util.indexing.FileBasedIndex; +import com.intellij.util.indexing.FileBasedIndexImpl; import com.intellij.util.lang.CompoundRuntimeException; import com.intellij.util.ui.UIUtil; import gnu.trove.Equality; @@ -52,6 +55,7 @@ import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.util.*; +import java.util.concurrent.TimeUnit; /** * @author peter @@ -141,6 +145,10 @@ public abstract class UsefulTestCase extends TestCase { // don't use method references here to make stack trace reading easier //noinspection Convert2MethodRef new RunAll( + () -> EdtTestUtil.runInEdtAndWait(() -> { + FileBasedIndexImpl index = ApplicationManager.getApplication() == null ? null : (FileBasedIndexImpl)FileBasedIndex.getInstance(); + if (index != null) index.waitForVfsEventsExecuted(1, TimeUnit.MINUTES); + }), () -> disposeRootDisposable(), () -> cleanupSwingDataStructures(), () -> cleanupDeleteOnExitHookList(), diff --git a/platform/testFramework/src/com/intellij/testFramework/fixtures/CodeInsightFixtureTestCase.java b/platform/testFramework/src/com/intellij/testFramework/fixtures/CodeInsightFixtureTestCase.java index 8a9f27af57f4..6ca37f072b06 100644 --- a/platform/testFramework/src/com/intellij/testFramework/fixtures/CodeInsightFixtureTestCase.java +++ b/platform/testFramework/src/com/intellij/testFramework/fixtures/CodeInsightFixtureTestCase.java @@ -1,6 +1,7 @@ // 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.testFramework.fixtures; +import com.intellij.codeInsight.daemon.impl.DaemonCodeAnalyzerImpl; import com.intellij.openapi.editor.Editor; import com.intellij.openapi.module.Module; import com.intellij.openapi.project.Project; @@ -12,6 +13,7 @@ import com.intellij.testFramework.builders.ModuleFixtureBuilder; import org.jetbrains.annotations.NonNls; import java.io.File; +import java.util.concurrent.TimeUnit; /** * @author yole @@ -42,6 +44,8 @@ public abstract class CodeInsightFixtureTestCase @Override protected void tearDown() throws Exception { + DaemonCodeAnalyzerImpl.waitForAllEditorsFinallyLoaded(getProject(), 10, TimeUnit.SECONDS); + myModule = null; try { myFixture.tearDown(); diff --git a/platform/testFramework/src/com/intellij/testFramework/fixtures/impl/BaseFixture.java b/platform/testFramework/src/com/intellij/testFramework/fixtures/impl/BaseFixture.java index 7b56807c3055..993374810983 100644 --- a/platform/testFramework/src/com/intellij/testFramework/fixtures/impl/BaseFixture.java +++ b/platform/testFramework/src/com/intellij/testFramework/fixtures/impl/BaseFixture.java @@ -17,13 +17,18 @@ package com.intellij.testFramework.fixtures.impl; import com.intellij.concurrency.IdeaForkJoinWorkerThreadFactory; import com.intellij.openapi.Disposable; +import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.util.Disposer; import com.intellij.testFramework.EdtTestUtil; import com.intellij.testFramework.UsefulTestCase; import com.intellij.testFramework.fixtures.IdeaTestFixture; +import com.intellij.util.indexing.FileBasedIndex; +import com.intellij.util.indexing.FileBasedIndexImpl; import org.jetbrains.annotations.NotNull; import org.junit.Assert; +import java.util.concurrent.TimeUnit; + /** * @author max */ @@ -47,6 +52,10 @@ public class BaseFixture implements IdeaTestFixture { public void tearDown() throws Exception { Assert.assertTrue("setUp() has not been called", myInitialized); Assert.assertFalse("tearDown() already has been called", myDisposed); + EdtTestUtil.runInEdtAndWait(() -> { + FileBasedIndexImpl index = ApplicationManager.getApplication() == null ? null : (FileBasedIndexImpl)FileBasedIndex.getInstance(); + if (index != null) index.waitForVfsEventsExecuted(1, TimeUnit.MINUTES); + }); disposeRootDisposable(); myDisposed = true; resetClassFields(getClass());