Commit df3101f1 authored by Sergey Prigogin's avatar Sergey Prigogin Committed by nik
Browse files

project model: fix memory leak of RootModelComponentBase instances via Disposer (IDEA-208450)

Objects that inherit from RootModelComponentBase get registered in CompositeDisposable of RootModelImpl. These objects are often disposed much sooner than RootModelImpl with methods like RootModelImpl.removeOrderEntry or RootModelImpl.removeContentEntry which explicitly call dispose on these objects.

This change removes objects extending RootModelComponentBase from the CompositeDisposable of RootModelImpl.
parent f8d66ed3
Branches unavailable Tags unavailable
No related merge requests found
Showing with 33 additions and 2 deletions
+33 -2
......@@ -41,7 +41,10 @@ public abstract class RootModelComponentBase implements Disposable {
@Override
public void dispose() {
myDisposed = true;
if (!myDisposed) {
myRootModel.unregisterOnDispose(this);
myDisposed = true;
}
}
public boolean isDisposed() {
......
......@@ -794,6 +794,10 @@ public class RootModelImpl extends RootModelBase implements ModifiableRootModel
myDisposable.add(disposable);
}
void unregisterOnDispose(@NotNull Disposable disposable) {
myDisposable.remove(disposable);
}
void checkModuleExtensionModification() {
if (myExtensionToStateDigest == null || myExtensionToStateDigest.isEmpty()) {
return;
......
......@@ -22,7 +22,7 @@ import java.util.ArrayList;
import java.util.List;
/**
* Like {@link com.intellij.openapi.Disposable}, you register instance of this class in {@link com.intellij.openapi.util.Disposer}.
* Like {@link Disposable}, you register instance of this class in {@link Disposer}.
* Call {@link #add(Disposable)} to request automatic disposal of additional objects.
* Comparing to registering these additional disposables with Disposer one by one,
* this class improves on the memory usage by not creating temporary objects inside Disposer.
......@@ -30,20 +30,44 @@ import java.util.List;
public class CompositeDisposable implements Disposable {
private final List<Disposable> myDisposables = new ArrayList<>();
private boolean disposed;
private boolean isDisposing;
public void add(@NotNull Disposable disposable) {
assert !disposed : "Already disposed";
myDisposables.add(disposable);
}
/**
* Removes the given disposable from this composite. Should be called when the disposable is disposed independently
* from the CompositeDisposable to prevent the CompositeDisposable from holding references to disposed objects.
* This method is safe to call while this CompositeDisposable is being disposed.
*
* @param disposable the disposable to remove from this CompositeDisposable
*/
public void remove(@NotNull Disposable disposable) {
// If isDisposing is true, there is no point of modifying the myDisposables list since it's going to be cleared anyway.
if (!isDisposing) {
for (int i = myDisposables.size() - 1; i >= 0; i--) {
Disposable d = myDisposables.get(i);
if (d == disposable) { // Compare by identity.
myDisposables.remove(i);
}
}
}
}
@Override
public void dispose() {
//assert !disposed : "Already disposed";
isDisposing = true;
for (int i = myDisposables.size() - 1; i >= 0; i--) {
Disposable disposable = myDisposables.get(i);
Disposer.dispose(disposable);
}
isDisposing = false;
myDisposables.clear();
disposed = true;
}
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment