GUAC-574: Synchronize across ActiveConnectionMap to prevent connection usage race conditions. Need to revisit architecture of connection management within MySQL.

extensions/guacamole-auth-mysql/src/main/java/net/sourceforge/guacamole/net/auth/mysql/MySQLGuacamoleSocket.java

@@ -39,7 +39,7 @@ public class MySQLGuacamoleSocket implements GuacamoleSocket {
      * Injected ActiveConnectionMap which will contain all active connections.
      */
     @Inject
-    private ActiveConnectionMap activeConnectionSet;
+    private ActiveConnectionMap activeConnectionMap;
 
     /**
      * The wrapped socket.
@@ -67,7 +67,7 @@ public class MySQLGuacamoleSocket implements GuacamoleSocket {
      * @param connectionGroupID The ID of the balancing connection group that is
      *                          being connected to; null if not used.
      */
-    public void init(GuacamoleSocket socket, int connectionID, int userID, 
+    public void init(GuacamoleSocket socket,
             int historyID, Integer connectionGroupID) {
         this.socket = socket;
         this.historyID = historyID;
@@ -91,7 +91,10 @@ public class MySQLGuacamoleSocket implements GuacamoleSocket {
         socket.close();
 
         // Mark this connection as inactive
-        activeConnectionSet.closeConnection(historyID, connectionGroupID);
+        synchronized (activeConnectionMap) {
+            activeConnectionMap.closeConnection(historyID, connectionGroupID);
+        }
+
     }
 
     @Override

extensions/guacamole-auth-mysql/src/main/java/net/sourceforge/guacamole/net/auth/mysql/service/ConnectionGroupService.java

@@ -176,7 +176,7 @@ public class ConnectionGroupService {
      * Connect to the connection within the given group with the lowest number
      * of currently active users.
      *
-     * @param connection The group to load balance across.
+     * @param group The group to load balance across.
      * @param info The information to use when performing the connection
      *             handshake.
      * @param userID The ID of the user who is connecting to the socket.
@@ -191,31 +191,36 @@ public class ConnectionGroupService {
         List<Integer> connectionIDs = connectionService.getAllConnectionIDs
                 (group.getConnectionGroupID());
         
-        // Get the least used connection.
+        synchronized (activeConnectionMap) {
-        Integer leastUsedConnectionID = 
-                activeConnectionMap.getLeastUsedConnection(connectionIDs);
 
-        if(leastUsedConnectionID == null)
+            // Get the least used connection.
-            throw new GuacamoleResourceNotFoundException("No connections found in group.");
+            Integer leastUsedConnectionID = 
+                    activeConnectionMap.getLeastUsedConnection(connectionIDs);
             
-        if(GuacamoleProperties.getProperty(
+            if(leastUsedConnectionID == null)
-                MySQLGuacamoleProperties.MYSQL_DISALLOW_SIMULTANEOUS_CONNECTIONS, false)
+                throw new GuacamoleResourceNotFoundException("No connections found in group.");
-                && activeConnectionMap.isActive(leastUsedConnectionID))
-            throw new GuacamoleServerBusyException
-                    ("Cannot connect. All connections are in use.");
             
-        if(GuacamoleProperties.getProperty(
+            if(GuacamoleProperties.getProperty(
-                MySQLGuacamoleProperties.MYSQL_DISALLOW_DUPLICATE_CONNECTIONS, true)
+                    MySQLGuacamoleProperties.MYSQL_DISALLOW_SIMULTANEOUS_CONNECTIONS, false)
-                && activeConnectionMap.isConnectionGroupUserActive(group.getConnectionGroupID(), userID))
+                    && activeConnectionMap.isActive(leastUsedConnectionID))
-            throw new GuacamoleClientTooManyException
+                throw new GuacamoleServerBusyException
-                    ("Cannot connect. Connection group already in use by this user.");
+                        ("Cannot connect. All connections are in use.");
             
-        // Get the connection 
+            if(GuacamoleProperties.getProperty(
-        MySQLConnection connection = connectionService
+                    MySQLGuacamoleProperties.MYSQL_DISALLOW_DUPLICATE_CONNECTIONS, true)
-                .retrieveConnection(leastUsedConnectionID, userID);
+                    && activeConnectionMap.isConnectionGroupUserActive(group.getConnectionGroupID(), userID))
+                throw new GuacamoleClientTooManyException
+                        ("Cannot connect. Connection group already in use by this user.");
+
+            // Get the connection 
+            MySQLConnection connection = connectionService
+                    .retrieveConnection(leastUsedConnectionID, userID);
+            
+            // Connect to the connection
+            return connectionService.connect(connection, info, userID, group.getConnectionGroupID());
+
+        }
             
-        // Connect to the connection
-        return connectionService.connect(connection, info, userID, group.getConnectionGroupID());
     }
     
     /**

extensions/guacamole-auth-mysql/src/main/java/net/sourceforge/guacamole/net/auth/mysql/service/ConnectionService.java

@@ -322,46 +322,49 @@ public class ConnectionService {
             GuacamoleClientInformation info, int userID, Integer connectionGroupID)
         throws GuacamoleException {
 
-        // If the given connection is active, and multiple simultaneous
+        synchronized (activeConnectionMap) {
-        // connections are not allowed, disallow connection
-        if(GuacamoleProperties.getProperty(
-                MySQLGuacamoleProperties.MYSQL_DISALLOW_SIMULTANEOUS_CONNECTIONS, false)
-                && activeConnectionMap.isActive(connection.getConnectionID()))
-            throw new GuacamoleResourceConflictException("Cannot connect. This connection is in use.");
 
-        if(GuacamoleProperties.getProperty(
+            // If the given connection is active, and multiple simultaneous
-                MySQLGuacamoleProperties.MYSQL_DISALLOW_DUPLICATE_CONNECTIONS, true)
+            // connections are not allowed, disallow connection
-                && activeConnectionMap.isConnectionUserActive(connection.getConnectionID(), userID))
+            if(GuacamoleProperties.getProperty(
-            throw new GuacamoleClientTooManyException
+                    MySQLGuacamoleProperties.MYSQL_DISALLOW_SIMULTANEOUS_CONNECTIONS, false)
-                    ("Cannot connect. Connection already in use by this user.");
+                    && activeConnectionMap.isActive(connection.getConnectionID()))
+                throw new GuacamoleResourceConflictException("Cannot connect. This connection is in use.");
             
-        // Get guacd connection information
+            if(GuacamoleProperties.getProperty(
-        String host = GuacamoleProperties.getRequiredProperty(GuacamoleProperties.GUACD_HOSTNAME);
+                    MySQLGuacamoleProperties.MYSQL_DISALLOW_DUPLICATE_CONNECTIONS, true)
-        int port = GuacamoleProperties.getRequiredProperty(GuacamoleProperties.GUACD_PORT);
+                    && activeConnectionMap.isConnectionUserActive(connection.getConnectionID(), userID))
+                throw new GuacamoleClientTooManyException
+                        ("Cannot connect. Connection already in use by this user.");
 
-        // Get socket
+            // Get guacd connection information
-        GuacamoleSocket socket;
+            String host = GuacamoleProperties.getRequiredProperty(GuacamoleProperties.GUACD_HOSTNAME);
-        if (GuacamoleProperties.getProperty(GuacamoleProperties.GUACD_SSL, false))
+            int port = GuacamoleProperties.getRequiredProperty(GuacamoleProperties.GUACD_PORT);
-            socket = new ConfiguredGuacamoleSocket(
-                new SSLGuacamoleSocket(host, port),
-                connection.getConfiguration(), info
-            );
-        else
-            socket = new ConfiguredGuacamoleSocket(
-                new InetGuacamoleSocket(host, port),
-                connection.getConfiguration(), info
-            );
 
-        // Mark this connection as active
+            // Get socket
-        int historyID = activeConnectionMap.openConnection(connection.getConnectionID(), 
+            GuacamoleSocket socket;
-                userID, connectionGroupID);
+            if (GuacamoleProperties.getProperty(GuacamoleProperties.GUACD_SSL, false))
+                socket = new ConfiguredGuacamoleSocket(
+                    new SSLGuacamoleSocket(host, port),
+                    connection.getConfiguration(), info
+                );
+            else
+                socket = new ConfiguredGuacamoleSocket(
+                    new InetGuacamoleSocket(host, port),
+                    connection.getConfiguration(), info
+                );
 
-        // Return new MySQLGuacamoleSocket
+            // Mark this connection as active
-        MySQLGuacamoleSocket mySQLGuacamoleSocket = mySQLGuacamoleSocketProvider.get();
+            int historyID = activeConnectionMap.openConnection(connection.getConnectionID(), 
-        mySQLGuacamoleSocket.init(socket, connection.getConnectionID(), userID, 
+                    userID, connectionGroupID);
-                historyID, connectionGroupID);
 
-        return mySQLGuacamoleSocket;
+                // Return new MySQLGuacamoleSocket
+            MySQLGuacamoleSocket mySQLGuacamoleSocket = mySQLGuacamoleSocketProvider.get();
+            mySQLGuacamoleSocket.init(socket, historyID, connectionGroupID);
+                
+            return mySQLGuacamoleSocket;
+
+        }
 
     }