[Spread-cvs] commit: r451 - in trunk: daemon java/spread

jonathan at spread.org jonathan at spread.org
Mon Jan 9 00:53:22 EST 2012


Author: jonathan
Date: 2012-01-09 00:53:22 -0500 (Mon, 09 Jan 2012)
New Revision: 451

Modified:
   trunk/daemon/Changelog
   trunk/java/spread/SpreadConnection.java
Log:
Fix several potential deadlocks in Spread Java library.

Modified: trunk/daemon/Changelog
===================================================================
--- trunk/daemon/Changelog	2012-01-09 03:02:59 UTC (rev 450)
+++ trunk/daemon/Changelog	2012-01-09 05:53:22 UTC (rev 451)
@@ -1,3 +1,11 @@
+
+Mon Jan  9 00:35:02 2012  Jonathan Stanton  <jonathan at spreadconcepts.com>
+
+	* SpreadConnection.java (run.listeners): Fix several potential
+	deadlocks and race condiditions in the Java client library handling
+	of removal of listeners and communciation between listeners and the
+	main thread. Bugs reported by Daniel Marques. 
+
 Sun Jan  8 21:55:37 2012  Jonathan Stanton  <jonathan at spreadconcepts.com>
 
 	* SpreadConnection.java (disconnect): Fix bug where calling 

Modified: trunk/java/spread/SpreadConnection.java
===================================================================
--- trunk/java/spread/SpreadConnection.java	2012-01-09 03:02:59 UTC (rev 450)
+++ trunk/java/spread/SpreadConnection.java	2012-01-09 05:53:22 UTC (rev 451)
@@ -1501,7 +1501,7 @@
 	 * @param  listener  the listener to remove
 	 * @see  SpreadConnection#add(BasicMessageListener)
 	 */
-	public void remove(BasicMessageListener listener)
+        public void remove(BasicMessageListener listener)
 	{
 	synchronized (listenersynchro) {
 		// Are we in a listener callback?
@@ -1524,7 +1524,7 @@
 		
 		// Check if we're connected.
 		////////////////////////////
-		if(connected == true)
+		if (connected == true)
 		{
 			// Check if there are any more listeners.
 			/////////////////////////////////////////
@@ -1537,7 +1537,7 @@
 		}
 	}
 	}
-	
+
 	// Removes an advanced message listener.
 	////////////////////////////////////////
 	/**
@@ -1547,7 +1547,7 @@
 	 * @param  listener  the listener to remove
 	 * @see SpreadConnection#add(AdvancedMessageListener)
 	 */
-	public void remove(AdvancedMessageListener listener)
+        public void remove(AdvancedMessageListener listener)
 	{
 	synchronized (listenersynchro) {
 		// Are we in a listener callback?
@@ -1570,7 +1570,7 @@
 		
 		// Check if we're connected.
 		////////////////////////////
-		if(connected == true)
+		if (connected == true)
 		{
 			// Check if there are any more listeners.
 			/////////////////////////////////////////
@@ -1594,7 +1594,7 @@
 		
 		// If true, the connection wants the thread to stop.
 		////////////////////////////////////////////////////
-		protected boolean signal;
+		protected volatile boolean signal;
 		
 		// The constructor.
 		///////////////////
@@ -1608,6 +1608,7 @@
 			// Be a daemon.
 			///////////////
 			this.setDaemon(true);
+
 		}
 		
 		// The thread's entry point.
@@ -1644,27 +1645,27 @@
 				}
 				while(true)
 				{
-					// Get a lock on the connection.
-					////////////////////////////////
-					synchronized(connection)
-					{
-						// Do they want us to stop?
-						///////////////////////////
-						if(signal == true)
-						{
-						    // We're done.
-						    //////////////
-						    System.out.println("LISTENER: told to exit so returning");
-						    try {
-							connection.socket.setSoTimeout(previous_socket_timeout);
-						    }
-						    catch( SocketException e ) {
-							// just ignore for now 
-							System.out.println("socket error setting timeout" + e.toString() );
-						    }
-
-						    return;
-						}
+				    // Do they want us to stop?
+				    ///////////////////////////
+				    if(signal == true)
+				    {
+					// We're done.
+					//////////////
+					System.out.println("LISTENER: told to exit so returning");
+					try {
+					    connection.socket.setSoTimeout(previous_socket_timeout);
+					}
+					catch( SocketException e ) {
+					    // just ignore for now 
+					    System.out.println("socket error setting timeout" + e.toString() );
+					}
+					
+					return;
+				    }
+				    // Get a lock on the connection.
+				    ////////////////////////////////
+				    synchronized(connection)
+				    {
 							
 						// Get a message.
 						// WE WILL BLOCK HERE UNTIL DATA IS AVAILABLE
@@ -1783,10 +1784,10 @@
 									
 								// Remove it.
 								/////////////
-								connection.remove(basicListener);
+								basicListeners.removeElement(basicListener);
 
-									// Remove the listener from the Vector.
-									///////////////////////////////////////
+								// Remove the listener from the Vector.
+								///////////////////////////////////////
 								listenerBuffer.removeElementAt(0);
 							}
 							else if(command == BUFFER_REMOVE_ADVANCED)
@@ -1797,14 +1798,34 @@
 									
 								// Remove it.
 								/////////////
-								connection.remove(advancedListener);
+								advancedListeners.removeElement(advancedListener);
 								// Remove the listener from the Vector.
 								///////////////////////////////////////
 								listenerBuffer.removeElementAt(0);
 							}
 						}
+						// Check if there are any more listeners, if not, then quit listener thread. 
+						/////////////////////////////////////////
+						if((basicListeners.size() == 0) && (advancedListeners.size() == 0))
+						{
+						    // Terminate and return from this listener thread.
+						    ////////////////////////////
+						    System.out.println("LISTENER: no current listeners registered so terminate Listener thread.");
+						    try {
+							connection.socket.setSoTimeout(previous_socket_timeout);
+						    }
+						    catch( SocketException e ) {
+							// just ignore for now 
+							System.out.println("socket error setting timeout" + e.toString() );
+						    }
+						    
+						    listener = null;
+						    // Exit from running Listener Thread
+						    return;
+						}
+
 					}
-					
+
 					// There are no messages waiting, take a break.
 					///////////////////////////////////////////////
 					yield();




More information about the Spread-cvs mailing list