Identifying Disposables

Answered

Hi Team,I'm asking this in the context of an IntelliJ ToolWindow Plugin.

 Is it good to make all classes (leaving plugin services) Disposable and implement dispose with “Disposer.register(myDisposableParent,this);”?

 Or, 

I've the below Disposable implementation to use as a Parent. Can you highlight which ones (from the classes I listed down below ) should I be registering for disposal mandatorily? 

@Service(value = {Service.Level.APP, Service.Level.PROJECT})
public final class DisposableParent implements Disposable {

    @Override
    public void dispose() {
    }

    public static DisposableParent getInstance() {
        return ApplicationManager.getApplication().getService(DisposableParent.class);
    }

    public static DisposableParent getInstance(Project project) {
        return project.getService(DisposableParent.class);

    }
}

Classes (registered as Plugin Services with Project Level) - Per doc, this gets disposed during project close/plugin unload. 

Classes(not registered as Plugin Services) - Hope this gets garbage collected

→  EditorEventMulticasterEx's addFocusChangeListener(this is already taken care as it requires passing a disposable and I pass the MessageBus from the project)

→ Objects of Open API components: 

JBCefBrowser (I'm already registering for disposal)

DialogWrapper - The doc suggests to use DialogWrapper.getDisposable(). I'm invoking the dialogwrapper using EDT. I'm not sure what would be the right stage/when to dispose this. I doubt if this manual disposal is prone to affect the ongoing operation happening in EDT.  

Hence, Instead of myDialogWrapper.getDisposable().dispose(), I prefer to make the DialogWrapper as Disposable and implement dispose() with “Disposer.register(myDisposableParent,this);” Does this look good?

EditorCustomElementRenderer - I prefer to make this class Disposable and implement dispose() with “Disposer.register(myDisposableParent,this);” Does this look good?

Project, ToolWindow, ToolWindowManager, Editor, ActionSystem for ActionGroups and Actions - Any action items to take on these objects for disposal?

IDE Event Dispatcher(registered as a plugin service) is added to IDE Event Queue IdeEventQueue.getInstance().addPostprocessor(myEventDispatcher, project) . Anything else applicable for disposal here?

0
4 comments

Hi,

In general, making all classes disposable is not necessary. Only those classes that manage resources that need explicit cleanup (like listeners, UI components, or other heavyweight resources) should implement Disposable.

Classes (registered as Plugin Services with Project Level) - Per doc, this gets disposed during project close/plugin unload. 

Yes. No need to implement Disposable if you don't need to clean up resources.

Classes(not registered as Plugin Services) - Hope this gets garbage collected

Yes. No need to implement Disposable if you don't need to clean up resources.

JBCefBrowser (I'm already registering for disposal)

I don't understand what you do and why. Do you extend JBCefBrowser and override dispose? Do you set up any resource you need to clean up when the browser is disposed?

DialogWrapper - The doc suggests to use DialogWrapper.getDisposable(). I'm invoking the dialogwrapper using EDT. I'm not sure what would be the right stage/when to dispose this. I doubt if this manual disposal is prone to affect the ongoing operation happening in EDT.  

DialogWrapper.getDisposable should be used as a parent disposable when you register another disposable for cleaning resources. When a dialog is disposed, then its disposable's children will be disposed too. I don't understand what you mean by manual disposing.

Hence, Instead of myDialogWrapper.getDisposable().dispose(), I prefer to make the DialogWrapper as Disposable and implement dispose() with “Disposer.register(myDisposableParent,this);” Does this look good?

Sorry, I don't understand what's the idea here. Dialogs are disposed when they are closed and its getDisposable() allows registering children that will get disposed on dialog close too.

EditorCustomElementRenderer - I prefer to make this class Disposable and implement dispose() with “Disposer.register(myDisposableParent,this);” Does this look good?

No, registering disposables is usually done when an object is created/initialized, not in the dispose() method. The dispose() method should be used for cleaning up resources - removing listeners, closing connections, etc.

Project, ToolWindow, ToolWindowManager, Editor, ActionSystem for ActionGroups and Actions - Any action items to take on these objects for disposal?

Follow the general rule: use Disposable for classes that manage resources that need explicit cleanup.

IDE Event Dispatcher(registered as a plugin service) is added to IDE Event Queue IdeEventQueue.getInstance().addPostprocessor(myEventDispatcher, project) . Anything else applicable for disposal here?

Sorry, I don't understand the case.

In general, it seems to me that you could misunderstand the disposables purpose and how it works. I suggest carefully reading documentation: https://plugins.jetbrains.com/docs/intellij/disposers.html

0

Hi There! Thanks for reading my question over and responding back. 

Having your responses as the underlying idea, I did go over the documentation again and get some insights. Please find my answers to your questions in response to mine. 

  • JBCefBrowser - “I don't understand what you do and why. Do you extend JBCefBrowser and override dispose? Do you set up any resource you need to clean up when the browser is disposed? ”

    I have a Browser class to deal with JBCefBrowser and its related stuffs. JBCefBrowser is a member variable in the class. I use this browser to render contents in the ToolWindow. I do not have any resources to clean up specifically but the JBCefBrowser which I consider heavy weight. When I do not want the JBCefBrowser to outlive the project, do you find the below approach correct? 

public class Browser implements Disposable {
    private final JBCefBrowser jbCefBrowser;

    public Browser(Project project) {
        this.jbCefBrowser = JBCefBrowser.createBuilder().build();
        this.jbCefBrowser.getJBCefClient().setProperty(JBCefClient.Properties.JS_QUERY_POOL_SIZE, getCefJsQueryPoolSize());

        Disposer.register(getProjectAsDisposableParent(project), this);
    }

    @Override
    public void dispose() {
        if (this.jbCefBrowser==null || this.jbCefBrowser.isDisposed()) {
            return;
        }
        ApplicationManager.getApplication().invokeLater(this.jbCefBrowser::dispose);
    }
}
  • DialogWrapper - “DialogWrapper.getDisposable should be used as a parent disposable when you register another disposable for cleaning resources. When a dialog is disposed, then its disposable's children will be disposed too. I don't understand what you mean by manual disposing.”

     I do not have any resources to clean up. But, I wanted to dispose the wrapper.  I read “For resources with a shorter lifetime, create a disposable using Disposer.newDisposable() and dispose it manually using Disposable.dispose()  from the documentation and hence thought of needing to dispose it manually. Now that you mentioned it gets disposed when closed, I'll skip this.

 

0

Hi,

Regarding the browser case, getProjectAsDisposableParent(project) looks suspicious as a project should not be used as a disposable parent. See the red box in https://plugins.jetbrains.com/docs/intellij/disposers.html#choosing-a-disposable-parent.

0

Hi, Thanks for getting back. The actual project is not going to be used as the parent. Instead, I've a service registered at Project level. This service is going to be used as the parent. Please find the method's implementation highlighted below


@Service(value = {Service.Level.APP, Service.Level.PROJECT})
public final class DisposableParent implements Disposable {

    @Override
    public void dispose() {
    }
    public static DisposableParent getProjectAsDisposableParent(Project project) {
        return project.getService(DisposableParent.class);
    }

 public static DisposableParent getApplicationAsDisposableParent() {
        return ApplicationManager.getApplication().getService(DisposableParent.class);
    }

}
 

0

Please sign in to leave a comment.