Re: [EGIT PATCH 2/5] quickdiff - Don't add GitDocument as repository listener more than once

From: Shawn O. Pearce <spearce@spearce.org>
Date: Apr 5, 2009, 20:24 UTC
Message-ID: <20090405202400.GO23521@spearce.org>
In-Reply-To: <1238697991-12990-3-git-send-email-robin.rosenberg@dewire.com>

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:

> Doing so was very costly and led to sessions being locked
> for a long time while refreshing the reference document used
> by Eclipse's quickdiff feature.

...

Show 17 quoted lines

> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
> @@ -1132,6 +1132,7 @@ public File getWorkDir() {
>  	 * @param l
>  	 */
>  	public void addRepositoryChangedListener(final RepositoryListener l) {
> +		assert !listeners.contains(l);
>  		listeners.add(l);
>  	}
>  
> @@ -1150,6 +1151,7 @@ public void removeRepositoryChangedListener(final RepositoryListener l) {
>  	 * @param l
>  	 */
>  	public static void addAnyRepositoryChangedListener(final RepositoryListener l) {
> +		assert !allListeners.contains(l);
>  		allListeners.add(l);
>  	}

That's a race condition. The two collections are Vectors so another thread can add the listener between your assertion point and the add call.

Also, if this really is considered to be very bad behavior on the part of the application, maybe these should be real tests with exceptions thrown, so they can't be disabled when assertions are turned off in the JRE?

-- 
Shawn.