diff --git a/platform/platform-impl/codeinsight-inline/src/com/intellij/codeInsight/inline/completion/logs/InlineCompletionLogsContainer.kt b/platform/platform-impl/codeinsight-inline/src/com/intellij/codeInsight/inline/completion/logs/InlineCompletionLogsContainer.kt index 1d4eeb989eb0..8c0be0a0599c 100644 --- a/platform/platform-impl/codeinsight-inline/src/com/intellij/codeInsight/inline/completion/logs/InlineCompletionLogsContainer.kt +++ b/platform/platform-impl/codeinsight-inline/src/com/intellij/codeInsight/inline/completion/logs/InlineCompletionLogsContainer.kt @@ -37,8 +37,6 @@ class InlineCompletionLogsContainer() { private val forceFullLogs: AtomicBoolean = AtomicBoolean(false) - private var project: Project? = null - fun forceFullLogs() { forceFullLogs.set(true) } @@ -87,10 +85,6 @@ class InlineCompletionLogsContainer() { } } - fun addProject(project: Project?) { - this.project = project - } - /** * Use to add log to log container. * If you have to launch expensive computation and don't want to pause your main execution (especially if you are on EDT) use [addAsync]. @@ -119,8 +113,11 @@ class InlineCompletionLogsContainer() { * Cancel all [asyncAdds] and send current log container. * Await for this function completion before exit from the inline completion request and process next typings or next requests. * Should be very fast. + * + * [project] is passed explicitly instead of being stored in a field: this container lives in the editor user data and is captured + * by async logging jobs running on an application-level scope, so it must not retain a project (LLM-17026). */ - fun logCurrent(extraLogger: CustomRequestIdLogger? = null) { + fun logCurrent(project: Project?, extraLogger: CustomRequestIdLogger? = null) { cancelAsyncAdds() val shouldSendFullLogs = getShouldSendFullLogs() diff --git a/platform/platform-impl/codeinsight-inline/src/com/intellij/codeInsight/inline/completion/logs/InlineCompletionLogsListener.kt b/platform/platform-impl/codeinsight-inline/src/com/intellij/codeInsight/inline/completion/logs/InlineCompletionLogsListener.kt index 3c6e3a66e300..5f63493bb957 100644 --- a/platform/platform-impl/codeinsight-inline/src/com/intellij/codeInsight/inline/completion/logs/InlineCompletionLogsListener.kt +++ b/platform/platform-impl/codeinsight-inline/src/com/intellij/codeInsight/inline/completion/logs/InlineCompletionLogsListener.kt @@ -74,7 +74,6 @@ internal class InlineCompletionLogsListener(private val editor: Editor) : Inline holder.requestId = event.request.requestId val container = InlineCompletionLogsContainer.create(event.request.editor) - container.addProject(event.request.editor.project) container.add(REQUEST_ID with event.request.requestId) container.add(COMPLETION_ID with event.request.requestId.toString()) container.add(REQUEST_EVENT with event.request.event.javaClass) @@ -179,7 +178,8 @@ internal class InlineCompletionLogsListener(private val editor: Editor) : Inline } } } - container.logCurrent(CustomRequestIdLogger.remove(editor)) // see doc of this function, it's very fast, and we should wait for its completion + // see doc of this function, it's very fast, and we should wait for its completion + container.logCurrent(editor.project, CustomRequestIdLogger.remove(editor)) // `SELECTED` case is handled in the afterInsert case if (event.finishType != InlineCompletionUsageTracker.ShownEvents.FinishType.SELECTED) { diff --git a/platform/platform-tests/testSrc/com/intellij/codeInsight/inline/completion/logs/InlineCompletionLogsContainerTest.kt b/platform/platform-tests/testSrc/com/intellij/codeInsight/inline/completion/logs/InlineCompletionLogsContainerTest.kt index 41df32973c3e..b5808f661424 100644 --- a/platform/platform-tests/testSrc/com/intellij/codeInsight/inline/completion/logs/InlineCompletionLogsContainerTest.kt +++ b/platform/platform-tests/testSrc/com/intellij/codeInsight/inline/completion/logs/InlineCompletionLogsContainerTest.kt @@ -5,7 +5,9 @@ import com.intellij.codeInsight.inline.completion.InlineCompletionEapSupport import com.intellij.codeInsight.inline.completion.logs.InlineCompletionLogsContainer.Phase import com.intellij.internal.statistic.FUCollectorTestCase import com.intellij.internal.statistic.eventLog.events.EventFields +import com.intellij.openapi.project.Project import com.intellij.testFramework.ExtensionTestUtil +import com.intellij.testFramework.LeakHunter import com.intellij.testFramework.LightPlatformTestCase import com.intellij.testFramework.common.timeoutRunBlocking import org.junit.Test @@ -39,7 +41,7 @@ class InlineCompletionLogsContainerTest : LightPlatformTestCase() { logsContainer.add(TestPhasedLogs.fullTestField with 1337) val logs = FUCollectorTestCase.collectLogEvents(recorder = "ML", parentDisposable = testRootDisposable, escapeChars = true) { - logsContainer.logCurrent() + logsContainer.logCurrent(project = null) } // expect both logs @@ -67,7 +69,7 @@ class InlineCompletionLogsContainerTest : LightPlatformTestCase() { logsContainer.add(TestPhasedLogs.fullTestField with 1337) val logs = FUCollectorTestCase.collectLogEvents(recorder = "ML", parentDisposable = testRootDisposable, escapeChars = true) { - logsContainer.logCurrent() + logsContainer.logCurrent(project = null) } // expect both logs @@ -96,7 +98,7 @@ class InlineCompletionLogsContainerTest : LightPlatformTestCase() { logsContainer.add(TestPhasedLogs.fullTestField with 1337) val logs = FUCollectorTestCase.collectLogEvents(recorder = "ML", parentDisposable = testRootDisposable, escapeChars = true) { - logsContainer.logCurrent() + logsContainer.logCurrent(project = null) } // expect both logs @@ -157,7 +159,7 @@ class InlineCompletionLogsContainerTest : LightPlatformTestCase() { logsContainer.add(TestPhasedLogs.basicTestField with 99) val logs = FUCollectorTestCase.collectLogEvents(recorder = "ML", parentDisposable = testRootDisposable, escapeChars = true) { - logsContainer.logCurrent() + logsContainer.logCurrent(project = null) } assertMaps( @@ -170,6 +172,28 @@ class InlineCompletionLogsContainerTest : LightPlatformTestCase() { ) } + /** + * The container lives in the editor user data and is captured by async logging jobs running on an application-level scope, + * so it must never hold a strong reference to a project: the project is passed to [InlineCompletionLogsContainer.logCurrent] + * instead. See LLM-17026. + */ + @Test + fun testContainerDoesNotRetainProject() { + val logsContainer = InlineCompletionLogsContainer() + logsContainer.mockRandom(1f) + // Only a primitive field and no `addAsync` on purpose: `DebugReflectionUtil.isTrivial` treats only primitives, strings and + // arrays as trivial, so an `EventFields.Class` value would make LeakHunter walk the statics of the logged class, and a job + // left in `asyncAdds` would open a path into the application-level scope. + logsContainer.add(TestPhasedLogs.basicTestField with 42) + + val logs = FUCollectorTestCase.collectLogEvents(recorder = "ML", parentDisposable = testRootDisposable, escapeChars = true) { + logsContainer.logCurrent(project) + } + + assertNotNull("The project must still be reported in the FUS event", logs.first().event.data["project"]) + LeakHunter.checkLeak(logsContainer, Project::class.java) + } + private fun withEap(isEAP: Boolean, action: () -> Unit) { try { InlineCompletionEapSupport.getInstance().setMockEap(isEAP)