commit: r451 - in trunk: daemon java/spread
[email protected] Mon, 09 Jan 2012 00:53:22 -0500
| Newsgroups | gmane.network.spread.cvs |
|---|---|
| Message-ID | <[email protected]> |
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 <[email protected]> + + * 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 <[email protected]> * 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();