vsch / idea-multimarkdown

Markdown language support for IntelliJ IDEA.
https://plugins.jetbrains.com/plugin/7896-markdown-navigator
Apache License 2.0
813 stars 129 forks source link

Memory leak when project is closed due to MdProjectSettings #828

Open radomirm-g opened 4 years ago

radomirm-g commented 4 years ago

Android Studio runs memory analysis when memory is low. Below is a fragment of such report sent by a user:

Disposed but still strong-referenced objects: 14 com.intellij.openapi.project.impl.ProjectImpl, most common paths from GC-roots:
Root 1:
[   13/ 92%/1.32KB]  302MB          1   ROOT: Java Frame: com.intellij.openapi.progress.impl.CoreProgressManager.runProcessWithProgressSynchronously(CoreProgressManager.java:459)
[   13/ 92%/1.32KB]  302MB          1   (root): com.intellij.openapi.application.impl.ApplicationImpl
[   12/ 85%/1.22KB]  241MB          1   +-(disposer-tree): com.intellij.ide.ui.laf.LafManagerImpl
[   12/ 85%/1.22KB]  241MB          1   | myEventDispatcher: com.intellij.util.EventDispatcher
[   12/ 85%/1.22KB]  241MB          1   | myListeners: com.intellij.util.containers.DisposableWrapperList
[   12/ 85%/1.22KB]  241MB          1   | myWrappedList: com.intellij.util.containers.LockFreeCopyOnWriteArrayList
[   12/ 85%/1.22KB]  241MB          1   | array: java.lang.Object[]
[   12/ 85%/1.22KB]  241MB         12   | []: com.intellij.util.containers.DisposableWrapperList$DisposableWrapper
[   12/ 85%/1.22KB]  241MB         12 ! | delegate: com.vladsch.idea.multimarkdown.settings.MdProjectSettings (disposed)
[   12/ 85%/1.22KB]  241MB         12 * | myProject: com.intellij.openapi.project.impl.ProjectImpl (disposed)

The report points to a memory leak in Markdown Navigator plugin where ProjectImpl objects are leaked (which are large structures to closed projects). These objects are kept alive due to a reference in MdProjectSettings.myProject. MdProjectSettings is a Disposable object itself and has been already disposed.

To drill further, such disposed MdProjectSettings is kept alive due to LafManager.addLafManagerListener() call. GitHub doesn't show the code for the latest version, so the following is my best guess of what might be the cause.

Likely, the code that registers MdProjectSettings as a listener should use LafManager.addLafManagerListener(mdProjectSettings, mdProjectSettings) instead of LafManager.addLafManagerListener(mdProjectSettings). This will remove the listener once MdProjectSettings is disposed.

Another option is to explicitly call removeLafManagerListener in MdProjectSettings.dispose().

Repro is quite simple: open and then close project when MultiMarkdown plugin is enabled. You might need to edit a .md file, but I think it is not necessary to cause a leak. Observe the memory heap with a profiler (VisualVM, YourKit), you'll notice more and more ProjectImpl objects are being kept alive.

Verified with Markdown Navigator 2.9.7.

vsch commented 4 years ago

@radomirm-g, thank you for the catch. You are right, the listener needs to be registered with a disposable parent for automatic disposal.