harmony-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From telli...@apache.org
Subject svn commit: r418133 - in /incubator/harmony/enhanced/classlib/trunk/modules/beans/src: main/java/java/beans/ test/java/org/apache/harmony/beans/tests/java/beans/
Date Thu, 29 Jun 2006 20:44:31 GMT
Author: tellison
Date: Thu Jun 29 13:44:31 2006
New Revision: 418133

URL: http://svn.apache.org/viewvc?rev=418133&view=rev
Log:
Beans property change support should ensure the list of listeners
does not change during notification (else causes concurrent
modification exception).

Modified:
    incubator/harmony/enhanced/classlib/trunk/modules/beans/src/main/java/java/beans/PropertyChangeSupport.java
    incubator/harmony/enhanced/classlib/trunk/modules/beans/src/main/java/java/beans/VetoableChangeSupport.java
    incubator/harmony/enhanced/classlib/trunk/modules/beans/src/test/java/org/apache/harmony/beans/tests/java/beans/PropertyChangeSupportTest.java

Modified: incubator/harmony/enhanced/classlib/trunk/modules/beans/src/main/java/java/beans/PropertyChangeSupport.java
URL: http://svn.apache.org/viewvc/incubator/harmony/enhanced/classlib/trunk/modules/beans/src/main/java/java/beans/PropertyChangeSupport.java?rev=418133&r1=418132&r2=418133&view=diff
==============================================================================
--- incubator/harmony/enhanced/classlib/trunk/modules/beans/src/main/java/java/beans/PropertyChangeSupport.java
(original)
+++ incubator/harmony/enhanced/classlib/trunk/modules/beans/src/main/java/java/beans/PropertyChangeSupport.java
Thu Jun 29 13:44:31 2006
@@ -382,24 +382,32 @@
 		Object oldValue = event.getOldValue();
 		Object newValue = event.getNewValue();
 
-		if ((newValue != null) && (oldValue != null)
-				&& newValue.equals(oldValue)) {
+		if ((newValue != null) && (oldValue != null) && newValue.equals(oldValue))
{
 			return;
 		}
 
-		Iterator<PropertyChangeListener> iterator = allPropertiesChangeListeners.iterator();
-		while (iterator.hasNext()) {
-			PropertyChangeListener listener = (PropertyChangeListener) iterator
-					.next();
+		/* Copy the listeners collections so they can be modified while we fire events. */
+
+		PropertyChangeListener[] listensToAll;	// Listeners to all property change events
+		PropertyChangeListener[] listensToOne = null; // Listens to a given property change
+		synchronized(this) {
+			 listensToAll = allPropertiesChangeListeners.toArray(
+				new PropertyChangeListener[allPropertiesChangeListeners.size()]);
+			
+			List<PropertyChangeListener> listeners =
+				selectedPropertiesChangeListeners.get(propertyName);
+			if (listeners != null) {
+				listensToOne = listeners.toArray(
+					new PropertyChangeListener[listeners.size()]); 
+			}
+		}
+
+		// Fire the listeners
+		for (PropertyChangeListener	listener : listensToAll) {
 			listener.propertyChange(event);
 		}
-		List<PropertyChangeListener> listeners = selectedPropertiesChangeListeners
-				.get(propertyName);
-		if (listeners != null) {
-			iterator = listeners.iterator();
-			while (iterator.hasNext()) {
-				PropertyChangeListener listener = (PropertyChangeListener) iterator
-						.next();
+		if (listensToOne != null) {
+			for (PropertyChangeListener	listener : listensToAll) {
 				listener.propertyChange(event);
 			}
 		}

Modified: incubator/harmony/enhanced/classlib/trunk/modules/beans/src/main/java/java/beans/VetoableChangeSupport.java
URL: http://svn.apache.org/viewvc/incubator/harmony/enhanced/classlib/trunk/modules/beans/src/main/java/java/beans/VetoableChangeSupport.java?rev=418133&r1=418132&r2=418133&view=diff
==============================================================================
--- incubator/harmony/enhanced/classlib/trunk/modules/beans/src/main/java/java/beans/VetoableChangeSupport.java
(original)
+++ incubator/harmony/enhanced/classlib/trunk/modules/beans/src/main/java/java/beans/VetoableChangeSupport.java
Thu Jun 29 13:44:31 2006
@@ -295,48 +295,56 @@
     }
     
     private void doFirePropertyChange(PropertyChangeEvent event)
-            throws PropertyVetoException {
-        String propName = event.getPropertyName();
-        Object oldValue = event.getOldValue();
-        Object newValue = event.getNewValue();
-        
-        if (newValue != null && oldValue != null && newValue.equals(oldValue))
{
-            return;
-        }
-
-        try {
-            notifyAllListeners(event);
-        } catch (PropertyVetoException pve) {
-            PropertyChangeEvent rEvent = createPropertyChangeEvent(propName,
-                    newValue, oldValue);
-            notifyAllListeners(rEvent);
-            throw pve;
-        }
-    }
-
-    private void notifyListeners(Iterator<VetoableChangeListener> iterator, PropertyChangeEvent
event)
-            throws PropertyVetoException {
-
-        VetoableChangeListener listener = null;
-        while (iterator.hasNext()) {
-            listener = iterator.next();
-            listener.vetoableChange(event);
-        }
-
-    }
-
-    private void notifyAllListeners(PropertyChangeEvent event)
-            throws PropertyVetoException {
-
-        notifyListeners(allVetoableChangeListeners.iterator(), event);
-        String propertyName = event.getPropertyName();
-        List<VetoableChangeListener> listeners = selectedVetoableChangeListeners.get(propertyName);
-        if (listeners == null) {
-            return;
-        }
-        notifyListeners(listeners.iterator(), event);
-    }
-    
+			throws PropertyVetoException {
+		String propName = event.getPropertyName();
+		Object oldValue = event.getOldValue();
+		Object newValue = event.getNewValue();
+
+		if (newValue != null && oldValue != null && newValue.equals(oldValue))
{
+			return;
+		}
+
+		/* Take note of who we are going to notify (and potentially un-notify) */
+
+		VetoableChangeListener[] listensToAll; // Listeners to all property change events
+		VetoableChangeListener[] listensToOne = null; // Listens to a given property change
+		synchronized (this) {
+			listensToAll = allVetoableChangeListeners.toArray(
+				new VetoableChangeListener[allVetoableChangeListeners.size()]);
+
+			List<VetoableChangeListener> listeners =
+				selectedVetoableChangeListeners.get(event.getPropertyName());
+			if (listeners != null) {
+				listensToOne = listeners.toArray(
+					new VetoableChangeListener[listeners.size()]);
+			}
+		}
+
+		try {
+			for (VetoableChangeListener listener : listensToAll) {
+				listener.vetoableChange(event);
+			}
+			if (listensToOne != null) {
+				for (VetoableChangeListener listener : listensToOne) {
+					listener.vetoableChange(event);
+				}
+			}
+		} catch (PropertyVetoException pve) {
+			// Tell them we have changed it back
+			PropertyChangeEvent revertEvent = createPropertyChangeEvent(
+					propName, newValue, oldValue);
+			for (VetoableChangeListener listener : listensToAll) {
+				listener.vetoableChange(revertEvent);
+			}
+			if (listensToOne != null) {
+				for (VetoableChangeListener listener : listensToOne) {
+					listener.vetoableChange(revertEvent);
+				}
+			}
+			throw pve;
+		}
+	}
+
     private static VetoableChangeListener[] getAsVetoableChangeListenerArray(
             List<VetoableChangeListener> listeners) {
         Object[] objects = listeners.toArray();

Modified: incubator/harmony/enhanced/classlib/trunk/modules/beans/src/test/java/org/apache/harmony/beans/tests/java/beans/PropertyChangeSupportTest.java
URL: http://svn.apache.org/viewvc/incubator/harmony/enhanced/classlib/trunk/modules/beans/src/test/java/org/apache/harmony/beans/tests/java/beans/PropertyChangeSupportTest.java?rev=418133&r1=418132&r2=418133&view=diff
==============================================================================
--- incubator/harmony/enhanced/classlib/trunk/modules/beans/src/test/java/org/apache/harmony/beans/tests/java/beans/PropertyChangeSupportTest.java
(original)
+++ incubator/harmony/enhanced/classlib/trunk/modules/beans/src/test/java/org/apache/harmony/beans/tests/java/beans/PropertyChangeSupportTest.java
Thu Jun 29 13:44:31 2006
@@ -1353,6 +1353,39 @@
         public void propertyChange(PropertyChangeEvent event) {}
     }
 
+    /*
+     * Mock PropertyChangeListener that modifies the listener set on notification. 
+     */
+    static class MockPropertyChangeListener3 implements PropertyChangeListener {
+
+    	PropertyChangeSupport changeSupport;
+    	
+    	public MockPropertyChangeListener3(PropertyChangeSupport changeSupport) {
+    		super();
+    		this.changeSupport = changeSupport;
+    	}
+    	
+    	/* On property changed event modify the listener set */
+		public void propertyChange(PropertyChangeEvent event) {
+			changeSupport.addPropertyChangeListener(
+				new PropertyChangeListener(){
+					public void propertyChange(PropertyChangeEvent event) {
+						// Empty							
+					}
+				}
+			);
+		}
+    }
+
+    /**
+     * Regression test for concurrent modification of listener set 
+     */
+    public void testConcurrentModification() {
+    	PropertyChangeSupport changeSupport = new PropertyChangeSupport("bogus");
+    	MockPropertyChangeListener3 changeListener = new MockPropertyChangeListener3(changeSupport);
+    	changeSupport.firePropertyChange("bogus property", "previous", "newer");
+    }
+    
     /**
      * @tests java.beans.PropertyChangeSupport#PropertyChangeSupport(
      *        java.lang.Object)



Mime
View raw message