Shawn O. Pearce wrote: > Tor Arne Vestbø <torarnv@xxxxxxxxx> wrote: >> diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/Activator.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/Activator.java >> index fced643..d4a9e8e 100644 >> --- a/org.spearce.egit.ui/src/org/spearce/egit/ui/Activator.java >> +++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/Activator.java >> @@ -44,9 +48,24 @@ >> * This is a plugin singleton mostly controlling logging. >> */ >> public class Activator extends AbstractUIPlugin { >> + >> + /** >> + * The one and only instance >> + */ >> private static Activator plugin; > > What does this field do? I don't see it referenced anywhere... It's the singleton instance, referenced from Activator.getDefault(). I just documented it :) >> /** >> + * Property listeners for plugin specific events >> + */ >> + private static List<IPropertyChangeListener> propertyChangeListeners = >> + new ArrayList<IPropertyChangeListener>(5); > > None of these list accesses are thread-safe. Are we certain they > call will come from a single thread, e.g. the SWT event thread? > Or do we need to put synchronized protection in here? The addPropertyChangeListener method is called at startup from the GitLightweightDecorator constructor, in one of the worker threads, and same thing with removePropertyChangeListener from dispose(). The broadcastPropertyChange method is called in the main thread every time the Git decorator preference page is closes. This is the sync model used by other Eclipse plugins for keeping track of propertyChangeListeners, for example in the TeamUIPlugin, that's why I assumed it was OK. I'm perfectly fine with syncrhronizing it though, similar to repositoryChangeListeners in GitProjectData? Tor Arne -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html