Skip to content

fix: Ensure DocumentContentSynchronizer is always disposed#1425

Merged
angelozerr merged 1 commit intoredhat-developer:mainfrom
eliseevh:fix-dispose
Feb 24, 2026
Merged

fix: Ensure DocumentContentSynchronizer is always disposed#1425
angelozerr merged 1 commit intoredhat-developer:mainfrom
eliseevh:fix-dispose

Conversation

@eliseevh
Copy link
Contributor

Fixes #1360

@angelozerr
Copy link
Contributor

Thanks so much @eliseevh !

Let me try your PR

if (synchronizer != null) {
synchronizer.getDocument().removeDocumentListener(synchronizer);
synchronizer.dispose();
Disposer.dispose(synchronizer);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why you have removed removeDocumentListener ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because I changed call to addDocumentListener, so it would be automatically removed when synchronizer is disposed (it is important to call Disposer.dispose instead of directly calling dispose method on synchronizer). This way, document listener would be also removed if synchronizer is disposed in other places

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am sorry I don't understand very well.

document.addDocumentListener(synchronizer, synchronizer); is done but what about

document.removeDocumentListener(synchronizer);

You mean that line code Disposer.dispose(synchronizer); will remove synchronizer from document listener?

Copy link
Contributor Author

@eliseevh eliseevh Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. addDocumentListener(Document listener, Disposable) docs says that when second argument is disposed, it will remove document listener (you can look into implementation, it is probably something like addDocumentListener(listener); Disposer.register(disposable, () -> removeDocumentListener(listener));)
And if you register some disposable with Disposer.register(parent, child), when parent is disposed with Disposer.dispose, it will automatically dispose child, so it will remove document listener

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! I have verified this behavior.

Before calling Disposer.dispose:

image

After calling Disposer.dispose:

image

@angelozerr angelozerr merged commit 372c37c into redhat-developer:main Feb 24, 2026
6 checks passed
@angelozerr
Copy link
Contributor

Thanks so much @eliseevh !

@angelozerr angelozerr added the bug Something isn't working label Feb 24, 2026
@angelozerr angelozerr added this to the 0.20.0 milestone Feb 24, 2026
@angelozerr angelozerr moved this from Done to In Progress in Java Tooling Feb 24, 2026
@angelozerr angelozerr moved this from In Progress to Done in Java Tooling Feb 24, 2026
@angelozerr angelozerr removed the bug Something isn't working label Feb 24, 2026
@angelozerr angelozerr removed the status in Java Tooling Feb 24, 2026
@angelozerr angelozerr removed this from the 0.20.0 milestone Feb 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Memory leak detected in DocumentContentSynchronizer

2 participants