Upstream memory leak fix for ScopedPreferenceStore #2878
Conversation
introduce fix created by Sebastian Zarnekow: https://bugs.eclipse.org/bugs/show_bug.cgi?id=239033 https://bugs.eclipse.org/bugs/show_bug.cgi?id=362199 Also-By: Sebastian Zarnekow <[email protected]>
| @@ -224,8 +247,9 @@ private IEclipsePreferences getDefaultPreferences() { | |||
|
|
|||
| @Override | |||
| public void addPropertyChangeListener(IPropertyChangeListener listener) { | |||
| initializePreferencesListener();// Create the preferences listener if it | |||
| // does not exist | |||
| // Create the preference listeners if they do not exist | |||
There was a problem hiding this comment.
It looks like the life-cycle of the listener is solely bound to the IPropertyChangeListener, I therfore think it would be better to instead of handling it "globally" to instead have a wrapped listener object that is forwarding the events directly instead of holding an own instance in the store.
There was a problem hiding this comment.
What do you mean with a wrapped listener object?
It isn't dependent on a single IPropertyChangeListener but on the fact if any listeners are attached at all - no use listening if there is nothing to forward it to.
This copies over the relevant changes from: https://github.com/eclipse-xtext/xtext/blob/main/org.eclipse.xtext.ui/src/org/eclipse/xtext/ui/editor/preferences/FixedScopedPreferenceStore.java
There was a problem hiding this comment.
It isn't dependent on a single IPropertyChangeListener but on the fact if any listeners are attached at all
Instead of one "global" listener used for all IPropertyChangeListener it should wrap this one into the required type and use this for the life-cycle management.
|
@pazi146 I have now created a PR can you check if it solves your observed problem? |
introduce fix created by Sebastian Zarnekow:
https://bugs.eclipse.org/bugs/show_bug.cgi?id=239033
https://bugs.eclipse.org/bugs/show_bug.cgi?id=362199
Also-By: Sebastian Zarnekow [email protected]
Context: #2875