codota / tabnine-intellij

Jetbrains IDEs client for TabNine. Compatible with all IntelliJ-based IDEs. https://plugins.jetbrains.com/plugin/12798-tabnine
https://www.tabnine.com/install/intellij
MIT License
525 stars 67 forks source link

com.tabnine.intellij.completions retains disposed project #706

Closed ViugiNick closed 11 months ago

ViugiNick commented 11 months ago

Lambda in TabnineStatusBarWidget holds disposed TabnineStatusBarWidget and disposed ProjectImpl which leads to a memory leak

23.7MB/1 objects          (root): java.lang.Class(com.intellij.codeInsight.completion.CompletionContributor)
23.7MB/1 objects          (static): com.intellij.codeInsight.completion.CompletionExtension
23.7MB/1 objects          myCache: java.util.concurrent.ConcurrentHashMap
23.7MB/1 objects          table: [Ljava.util.concurrent.ConcurrentHashMap$Node;
23.7MB/1 objects          []: java.util.concurrent.ConcurrentHashMap$Node
23.7MB/1 objects          val: com.intellij.util.containers.ContainerUtil$ImmutableListBackedByArray
23.7MB/1 objects          myStore: [Ljava.lang.Object;
23.7MB/1 objects          []: com.tabnine.intellij.completions.TabNineCompletionContributor
23.7MB/1 objects          messageBus: com.intellij.util.messages.impl.RootBus
  232B/1 objects          subscribers: java.util.concurrent.ConcurrentLinkedQueue
  208B/1 objects          head: java.util.concurrent.ConcurrentLinkedQueue$Node
  104B/1 objects   (rep)  next: java.util.concurrent.ConcurrentLinkedQueue$Node
217MB/10 objects          item: com.intellij.util.messages.impl.MessageBusConnectionImpl
217MB/10 objects          subscriptions: java.util.concurrent.atomic.AtomicReference
217MB/10 objects          value: [Ljava.lang.Object;
217MB/10 objects          []: com.tabnine.statusBar.TabnineStatusBarWidget$$Lambda$2047/0x0000000101654ff8
217MB/10 objects          arg$1: com.tabnine.statusBar.TabnineStatusBarWidget(disposed)
217MB/10 objects          myProject: com.intellij.openapi.project.impl.ProjectImpl(disposed)

gz#28404

(related to Zendesk ticket #28404)

TzufTabnine commented 11 months ago

@ViugiNick Please send us the logs files, see here how to do that

ViugiNick commented 11 months ago

This bug is based on the Android Studio memory usage reporting, so we dont have access to idea.log files.

TzufTabnine commented 11 months ago

@ViugiNick Can you send us the engine log file? see here how to do that

ViugiNick commented 11 months ago

@TzufTabnine Unfortunately we don't have a scenario to reproduce this problem locally, so we can't attach the engine log file too. I think I found the place where this memory leak may happen:

ApplicationManager.getApplication()
        .getMessageBus()
        .connect(this)
        .subscribe(
            LimitedSecletionsChangedNotifier.LIMITED_SELECTIONS_CHANGED_TOPIC,
            limited -> {
              this.isLimited = limited;
              update();
            });

Here the registered message handler holds a reference to a TabnineStatusBarWidget that may become disposed before before the message is processed.

ofekby commented 11 months ago

@TzufTabnine Unfortunately we don't have a scenario to reproduce this problem locally, so we can't attach the engine log file too. I think I found the place where this memory leak may happen:

ApplicationManager.getApplication()
        .getMessageBus()
        .connect(this)
        .subscribe(
            LimitedSecletionsChangedNotifier.LIMITED_SELECTIONS_CHANGED_TOPIC,
            limited -> {
              this.isLimited = limited;
              update();
            });

Here the registered message handler holds a reference to a TabnineStatusBarWidget that may become disposed before before the message is processed.

Can't say that you are wrong, but I am almost sure that the subscribed handlers to a connection are disposed/garbage collected once the connection was disposed.

From the documentation of connect

Allows to create new connection that is bound to the given Disposable. That means that returned connection will be automatically released if given disposable parent is collected.
Params:
parentDisposable – target parent disposable to which life cycle newly created connection shall be bound
ViugiNick commented 11 months ago

@ofekby Thanks for your comment. You are absolutely right, in that case it seems like in this case disposed MessageBusConnectionImpl should be removed from the subscribers queue. But according to the reports it remained there for an extra hour. Will file a youtrack bug for it