Fixed deadlock that could freeze entire JVM. JM-1372

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

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

@@ -216,8 +216,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));
@@ -257,10 +255,6 @@ public class IQAdminHandler {
                         reply.setError(PacketError.Condition.bad_request);
                     }
                 }
-                    finally {
-                        room.lock.writeLock().unlock();
-                    }
-                }
                 catch (UserNotFoundException e) {
                     // Do nothing
                 }

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

@@ -20,10 +20,10 @@ import org.jivesoftware.openfire.forms.DataForm;
 import org.jivesoftware.openfire.forms.FormField;
 import org.jivesoftware.openfire.forms.spi.XDataFormImpl;
 import org.jivesoftware.openfire.forms.spi.XFormFieldImpl;
+import org.jivesoftware.openfire.muc.CannotBeInvitedException;
 import org.jivesoftware.openfire.muc.ConflictException;
 import org.jivesoftware.openfire.muc.ForbiddenException;
 import org.jivesoftware.openfire.muc.MUCRole;
-import org.jivesoftware.openfire.muc.CannotBeInvitedException;
 import org.jivesoftware.openfire.muc.cluster.RoomUpdatedEvent;
 import org.jivesoftware.openfire.user.UserNotFoundException;
 import org.jivesoftware.util.LocaleUtils;
@@ -240,7 +240,6 @@ public class IQOwnerHandler {
                 }
 
                 room.lock.readLock().unlock();
-                room.lock.writeLock().lock();
                 try {
                     for (String bareJID : jids.keySet()) {
                         String targetAffiliation = jids.get(bareJID);
@@ -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<Presence> presences = new ArrayList<Presence>(admins.size() + owners.size());
 
-        room.lock.writeLock().lock();
-        try {
         field = completedForm.getField("muc#roomconfig_roomname");
         if (field != null) {
             values = field.getValues();
@@ -540,11 +536,6 @@ public class IQOwnerHandler {
             room.destroyRoom(null, null);
         }
 
-        }
-        finally {
-            room.lock.writeLock().unlock();
-        }
-
         // Send the updated presences to the room occupants
         for (Object presence : presences) {
             room.send((Presence) presence);

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

@@ -1233,6 +1233,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();
@@ -1260,6 +1262,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,
@@ -1277,6 +1283,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();
@@ -1308,6 +1316,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,
@@ -1325,6 +1337,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()) {
@@ -1371,6 +1385,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
@@ -1392,6 +1410,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()) {
@@ -1406,6 +1426,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();
@@ -1430,25 +1474,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;
     }
 
@@ -1458,6 +1483,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()) {
@@ -1467,9 +1496,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;
@@ -1485,7 +1512,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;

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

@@ -286,14 +286,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")),