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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Tor Arne Vestbø <torarnv@xxxxxxxxx> 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.
--
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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux