Re: [EGIT PATCH 02/11] Use Set instead of array to keep track of change listeners

["Shawn O. Pearce" <spearce@spearce.org>]

Tor Arne Vestbø <torarnv@gmail.com> wrote:
> +	public static synchronized void removeRepositoryChangeListener(
> +			final RepositoryChangeListener objectThatCares) {
> +		repositoryChangeListeners.remove(objectThatCares);
>  	}
>  
>  	/**
> @@ -131,13 +133,8 @@ public static synchronized void addRepositoryChangeListener(
>  	 *            the repository which has had changes occur within it.
>  	 */
>  	static void fireRepositoryChanged(final RepositoryMapping which) {
> -		final RepositoryChangeListener[] e = getRepositoryChangeListeners();
> -		for (int k = e.length - 1; k >= 0; k--)
> -			e[k].repositoryChanged(which);
> -	}
> -
> -	private static synchronized RepositoryChangeListener[] getRepositoryChangeListeners() {
> -		return repositoryChangeListeners;
> +		for (RepositoryChangeListener listener : repositoryChangeListeners)
> +			listener.repositoryChanged(which);

See anything wrong here, like that the Set can be modified while
GitProjectData's class lock is held, but its being read here without
any locking?

The array trick worked before because we always made a copy anytime
the array was modified.  So we could safely return the array to the
caller and let the caller iterate it unlocked; we just had to read
the current array using a synchronized method to ensure we had a
stable read.

You'll need to copy the Set somehow while inside of a synchronized
method, then return the copy to the fireRepositoryChanged() method
so it can iterate the copy to fire the events.

-- 
Shawn.