[jitsi-dev] ConfigurationService.addPropertyChangeListener


#1

Hey

ConfigurationService.addPropertyChangeListener is currently implemented using a normal Hashtable. As the Java spec does not guarantee to call the finalizer of an object, it is sometimes quite difficult or even impossible to unregister change listeners cleanly.

Does anyone have an objection to change the Hashtable to a WeakHashMap?

The change listeners aren't yet used a lot, so the implications would be limited.

Regards,
Ingo


#2

ConfigurationService.addPropertyChangeListener is currently implemented using a normal Hashtable. As the Java spec does not guarantee to call the finalizer of an object, it is sometimes quite difficult or even impossible to unregister change listeners cleanly.

I'm afraid I don't understand the problem. If the programmer cares
about unregistering a PropertyChangeListener, then shouldn't
ConfigurationService.removePropertyChangeListener be used? Is the
problem specific to Hashtable? Could you please clarify?

Does anyone have an objection to change the Hashtable to a WeakHashMap?

The WeakHashMap is weak with respect to the key which in the case is
the property name so I'm not sure I see how using it instead of the
Hashtable will be correct. Could you please explain?

···

On Wed, Aug 10, 2011 at 3:04 AM, Bauersachs Ingo <ingo.bauersachs@fhnw.ch> wrote:


#3

ConfigurationService.addPropertyChangeListener is currently implemented

using a normal Hashtable. As the Java spec does not guarantee to call the
finalizer of an object, it is sometimes quite difficult or even impossible
to unregister change listeners cleanly.

I'm afraid I don't understand the problem. If the programmer cares
about unregistering a PropertyChangeListener, then shouldn't
ConfigurationService.removePropertyChangeListener be used? Is the
problem specific to Hashtable? Could you please clarify?

If the programmer _knows_ that he is no longer using the notifications this is the right way to go. But often you do not know when you're no longer using a registered notification. Concrete example: A form implemented on top of the ConfigurationForm interface. The form is instantiated through the AdvancedConfigurationPanel. When you navigate away, the reference to the form is thrown away and now collectable through the GC.
Now assume the form registered a change listener. This listener stays in the Hashtable for as long as the whole application lives. There is no way for the ConfigurationForm to know when to call removePropertyListener.

Does anyone have an objection to change the Hashtable to a WeakHashMap?

The WeakHashMap is weak with respect to the key which in the case is
the property name so I'm not sure I see how using it instead of the
Hashtable will be correct. Could you please explain?

Right, my memory lost me. I meant e.g. org.apache.commons.collections.map.ReferenceMap


#4

The ConfigurationForm infrastructure imposes this problem not only on
ConfigurationService and its Hashtable but on just about any listener.
For example, I've had this problem with the video preview because I
didn't know that the video preview was to be stopped when the user
navigated away from the Video ConfigurationForm or closed the Tools >
Options dialog. In my case I resorted to multiple ways of the
detecting that the ConfigurationForm was no longer displayed (and
then, of course, that it was later displayed again). What if we
extended the ConfigurationForm in general to be notified when it is
hidden/shown/paused/resumed/whatever and thus made it possible for
extenders to clean up their listeners?

···

On Wed, Aug 10, 2011 at 11:08 AM, Bauersachs Ingo <ingo.bauersachs@fhnw.ch> wrote:

If the programmer _knows_ that he is no longer using the notifications this is the right way to go. But often you do not know when you're no longer using a registered notification. Concrete example: A form implemented on top of the ConfigurationForm interface. The form is instantiated through the AdvancedConfigurationPanel. When you navigate away, the reference to the form is thrown away and now collectable through the GC.
Now assume the form registered a change listener. This listener stays in the Hashtable for as long as the whole application lives. There is no way for the ConfigurationForm to know when to call removePropertyListener.


#5

The ConfigurationForm infrastructure imposes this problem not only on
ConfigurationService and its Hashtable but on just about any listener.
For example, I've had this problem with the video preview because I
didn't know that the video preview was to be stopped when the user
navigated away from the Video ConfigurationForm or closed the Tools >
Options dialog. In my case I resorted to multiple ways of the
detecting that the ConfigurationForm was no longer displayed (and
then, of course, that it was later displayed again). What if we
extended the ConfigurationForm in general to be notified when it is
hidden/shown/paused/resumed/whatever and thus made it possible for
extenders to clean up their listeners?

Well, that might be a good idea for the ConfigurationForm anyway, especially for you video example. For the ConfigService though I'd prefer a weak map as it is 1) so easy to forget about, 2) not always possible to extend an interface to get notified (if it were, there would be nothing left to the GC) and 3) creates less plugin code (and the less code, the higher its reliability).