From: Shawn O. Pearce <spearce@spearce.org>
Date: Sun, 05 Apr 2009 20:24:00 GMT
Subject: Re: [EGIT PATCH 2/5] quickdiff - Don't add GitDocument as repository listener more than once
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.
...
> --- 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.