Fixed deadlock that could freeze entire JVM. JM-1372

git-svn-id: http://svn.igniterealtime.org/svn/repos/openfire/branches@10437 b35dd754-fafc-0310-a699-88a17e54d16e

openfire_3_5_2/src/java/org/jivesoftware/openfire/muc/spi/IQAdminHandler.java

@@ -223,8 +223,6 @@ public class IQAdminHandler {
                         jid = room.getOccupant(nick).getUserAddress();
                     }
 
-                    room.lock.writeLock().lock();
-                    try {
                     if ("moderator".equals(target)) {
                         // Add the user as a moderator of the room based on the full JID
                         presences.add(room.addModerator(jid, senderRole));
@@ -271,10 +269,6 @@ public class IQAdminHandler {
                         reply.setError(PacketError.Condition.bad_request);
                     }
                 }
-                    finally {
-                        room.lock.writeLock().unlock();
-                    }
-                }
                 catch (UserNotFoundException e) {
                     // Do nothing
                 }

openfire_3_5_2/src/java/org/jivesoftware/openfire/muc/spi/IQOwnerHandler.java

@@ -235,7 +235,6 @@ public class IQOwnerHandler {
                 }
 
                 room.lock.readLock().unlock();
-                room.lock.writeLock().lock();
                 try {
                     String targetAffiliation = null;
                     for (Iterator<String> it = jids.keySet().iterator(); it.hasNext();) {
@@ -266,7 +265,6 @@ public class IQOwnerHandler {
                     }
                 }
                 finally {
-                    room.lock.writeLock().unlock();
                     room.lock.readLock().lock();
                 }
             }
@@ -369,8 +367,6 @@ public class IQOwnerHandler {
         // Keep a registry of the updated presences
         List presences = new ArrayList(admins.size() + owners.size());
 
-        room.lock.writeLock().lock();
-        try {
         field = completedForm.getField("muc#roomconfig_roomname");
         if (field != null) {
             values = field.getValues();
@@ -541,11 +537,6 @@ public class IQOwnerHandler {
             room.destroyRoom(null, null);
         }
 
-        }
-        finally {
-            room.lock.writeLock().unlock();
-        }
-
         // Send the updated presences to the room occupants
         for (Iterator it = presences.iterator(); it.hasNext();) {
             room.send((Presence)it.next());

openfire_3_5_2/src/java/org/jivesoftware/openfire/muc/spi/LocalMUCRoom.java

@@ -1176,6 +1176,8 @@ public class LocalMUCRoom implements MUCRoom {
     }
 
     public List<Presence> addOwner(String bareJID, MUCRole sendRole) throws ForbiddenException {
+        lock.writeLock().lock();
+        try {
             MUCRole.Affiliation oldAffiliation = MUCRole.Affiliation.none;
             if (MUCRole.Affiliation.owner != sendRole.getAffiliation()) {
                 throw new ForbiddenException();
@@ -1203,6 +1205,10 @@ public class LocalMUCRoom implements MUCRoom {
                 null,
                 MUCRole.Affiliation.owner,
                 oldAffiliation);
+        }
+        finally {
+            lock.writeLock().unlock();
+        }
         // Update the presence with the new affiliation and inform all occupants
         try {
             return changeOccupantAffiliation(bareJID, MUCRole.Affiliation.owner,
@@ -1220,6 +1226,8 @@ public class LocalMUCRoom implements MUCRoom {
 
     public List<Presence> addAdmin(String bareJID, MUCRole sendRole) throws ForbiddenException,
             ConflictException {
+        lock.writeLock().lock();
+        try {
             MUCRole.Affiliation oldAffiliation = MUCRole.Affiliation.none;
             if (MUCRole.Affiliation.owner != sendRole.getAffiliation()) {
                 throw new ForbiddenException();
@@ -1251,6 +1259,10 @@ public class LocalMUCRoom implements MUCRoom {
                 null,
                 MUCRole.Affiliation.admin,
                 oldAffiliation);
+        }
+        finally {
+            lock.writeLock().unlock();
+        }
         // Update the presence with the new affiliation and inform all occupants
         try {
             return changeOccupantAffiliation(bareJID, MUCRole.Affiliation.admin,
@@ -1268,6 +1280,8 @@ public class LocalMUCRoom implements MUCRoom {
 
     public List<Presence> addMember(String bareJID, String nickname, MUCRole sendRole)
             throws ForbiddenException, ConflictException {
+        lock.writeLock().lock();
+        try {
             MUCRole.Affiliation oldAffiliation = (members.containsKey(bareJID) ?
                     MUCRole.Affiliation.member : MUCRole.Affiliation.none);
             if (isMembersOnly()) {
@@ -1314,6 +1328,10 @@ public class LocalMUCRoom implements MUCRoom {
                 nickname,
                 MUCRole.Affiliation.member,
                 oldAffiliation);
+        }
+        finally {
+            lock.writeLock().unlock();
+        }
         // Update other cluster nodes with new member
         CacheFactory.doClusterTask(new AddMember(this, bareJID, (nickname == null ? "" : nickname)));
         // Update the presence with the new affiliation and inform all occupants
@@ -1335,6 +1353,8 @@ public class LocalMUCRoom implements MUCRoom {
 
     public List<Presence> addOutcast(String bareJID, String reason, MUCRole senderRole)
             throws NotAllowedException, ForbiddenException, ConflictException {
+        lock.writeLock().lock();
+        try {
             MUCRole.Affiliation oldAffiliation = MUCRole.Affiliation.none;
             if (MUCRole.Affiliation.admin != senderRole.getAffiliation()
                     && MUCRole.Affiliation.owner != senderRole.getAffiliation()) {
@@ -1349,6 +1369,30 @@ public class LocalMUCRoom implements MUCRoom {
                 // Do nothing
                 return Collections.emptyList();
             }
+
+            // Update the affiliation lists
+            outcasts.add(bareJID);
+            // Remove the user from other affiliation lists
+            if (removeOwner(bareJID)) {
+                oldAffiliation = MUCRole.Affiliation.owner;
+            }
+            else if (removeAdmin(bareJID)) {
+                oldAffiliation = MUCRole.Affiliation.admin;
+            }
+            else if (removeMember(bareJID)) {
+                oldAffiliation = MUCRole.Affiliation.member;
+            }
+            // Update the DB if the room is persistent
+            MUCPersistenceManager.saveAffiliationToDB(
+                this,
+                bareJID,
+                null,
+                MUCRole.Affiliation.outcast,
+                oldAffiliation);
+        }
+        finally {
+            lock.writeLock().unlock();
+        }
         // Update the presence with the new affiliation and inform all occupants
         // actorJID will be null if the room itself (ie. via admin console) made the request
         JID actorJID = senderRole.getUserAddress();
@@ -1373,25 +1417,6 @@ public class LocalMUCRoom implements MUCRoom {
             // Effectively kick the occupant from the room
             kickPresence(presence, actorJID);
         }
-        // Update the affiliation lists
-        outcasts.add(bareJID);
-        // Remove the user from other affiliation lists
-        if (removeOwner(bareJID)) {
-            oldAffiliation = MUCRole.Affiliation.owner;
-        }
-        else if (removeAdmin(bareJID)) {
-            oldAffiliation = MUCRole.Affiliation.admin;
-        }
-        else if (removeMember(bareJID)) {
-            oldAffiliation = MUCRole.Affiliation.member;
-        }
-        // Update the DB if the room is persistent
-        MUCPersistenceManager.saveAffiliationToDB(
-            this,
-            bareJID,
-            null,
-            MUCRole.Affiliation.outcast,
-            oldAffiliation);
         return updatedPresences;
     }
 
@@ -1401,6 +1426,10 @@ public class LocalMUCRoom implements MUCRoom {
 
     public List<Presence> addNone(String bareJID, MUCRole senderRole) throws ForbiddenException,
             ConflictException {
+        List<Presence> updatedPresences = null;
+        boolean wasMember = false;
+        lock.writeLock().lock();
+        try {
             MUCRole.Affiliation oldAffiliation = MUCRole.Affiliation.none;
             if (MUCRole.Affiliation.admin != senderRole.getAffiliation()
                     && MUCRole.Affiliation.owner != senderRole.getAffiliation()) {
@@ -1410,9 +1439,7 @@ public class LocalMUCRoom implements MUCRoom {
             if (owners.contains(bareJID) && owners.size() == 1) {
                 throw new ConflictException();
             }
-        List<Presence> updatedPresences = null;
-        boolean wasMember = members.containsKey(bareJID) || admins.contains(bareJID) ||
-                owners.contains(bareJID);
+            wasMember = members.containsKey(bareJID) || admins.contains(bareJID) || owners.contains(bareJID);
             // Remove the user from ALL the affiliation lists
             if (removeOwner(bareJID)) {
                 oldAffiliation = MUCRole.Affiliation.owner;
@@ -1428,7 +1455,10 @@ public class LocalMUCRoom implements MUCRoom {
             }
             // Remove the affiliation of this user from the DB if the room is persistent
             MUCPersistenceManager.removeAffiliationFromDB(this, bareJID, oldAffiliation);
-
+        }
+        finally {
+            lock.writeLock().unlock();
+        }
         // Update the presence with the new affiliation and inform all occupants
         try {
             MUCRole.Role newRole;

openfire_3_5_2/src/java/org/jivesoftware/openfire/muc/spi/LocalMUCUser.java

@@ -285,14 +285,8 @@ public class LocalMUCUser implements MUCUser {
                                         // Add the user as a member of the room if the room is
                                         // members only
                                         if (room.isMembersOnly()) {
-                                            room.lock.writeLock().lock();
-                                            try {
                                             room.addMember(info.attributeValue("to"), null, role);
                                         }
-                                            finally {
-                                                room.lock.writeLock().unlock();
-                                            }
-                                        }
 
                                         // Send the invitation to the invitee
                                         room.sendInvitation(new JID(info.attributeValue("to")),