Room.removeUser not thread safe

[Sport11can]

We are handling the internal request InternalEventObject.EVENT_USER_LOST. We have setup a dynamic room manager to create and delete rooms as necessary.

In this instance, the EVENT_USER_LOST is triggered and we are checking to see if they room they left is empty, so we can delete it. We have already discovered (and reported and received an interim patch) the NegativeArraySizeException when calling Room.getAllUsersButOne (http://forums.smartfoxserver.com/viewto ... highlight=). We have seen that the user is asynchronously removed from the server/room they were in at sometime during the extension handling, but the exact timing is different everytime. Sometimes by the time we check the room the user was in, its size is 0 and we just destroy it. Other times we check the room and there is 1 user in the room, which is the current user who is going to leave - this is why we use the getAllUsersButOne so we can see if there is anyone else left in the room.

Because of this asynchronous remove, we have programmed this section of code quite defensively as the following code demonstrates:

Code: Select all

synchronized(ownerRooms)
{
   int userCount = room.howManyUsers();
   if (userCount == 0)
   {
      //Empty room, delete it
      cleanupRoom(ownerRooms, room, roomSubTypeId, ownerId);
      return false;
   }

   if ( user != null )
   {
      User usersRemaining[] = room.getAllUsersButOne(user.getUserId());

      if ( usersRemaining == null ||  ( userCount == 1 && usersRemaining.length == 0) )
      {
         // See if the user is still in the room
         // could have been removed since we last checked
         if (room.howManyUsers() == 1)
         {
            room.removeUser(user,true,true);
         }

         cleanupRoom(ownerRooms, room, roomSubTypeId, ownerId);
         return false;
      }
   }
}

But we occasionaly get the following error:

[ WARNING ] [id: 45] (Room.removeUser): Couldn't remove user: mmo_user1832:2760, from room: Private-7--9223372036854762552

What I think must be happening is that in the sfs Room.removeUser it checks if the user exists, locks the room, and then tries to remove the user, but they have already been asynchronously removed between the check and the lock event.

I think it should be, check if they exist, lock, check if they exist and then remove.

[Lapo]

The server has also its auto-room-management system, it removes rooms according the following rules:

- regular room: removed when userCount==0 and the room owner is not logged in the server anymore

- game room: removed as soon all players leave

In order to bypass this system and plug in your own, you should make sure that the ownership of dynamic rooms is attributed to the "Server", not a User.
This is done by passing a null as the "owner" in the createRoom call.

Are you already using this strategy in your application and tests?

[Sport11can]

Yes, when calling the ExtensionHelper createRoom() we are passing null as the owner.

[Lapo]

Ok, thanks. We'll setup a local test and try reproduce it.

One more questions. It seems that you're basically re-creating the same auto-destroy functionality that's already built into the server.
Why not simply letting the room being auto-destroyed when the last user goes out? You can anyways apply your custom logic by using events.

[Lapo]

One additional note.
You should retry your test with the <DeadChannelsPolicy> setting set to NORMAL. I wouldn't expect the problem to appear again, if I remember correctly.

Let me know how it goes.

[ilovemavis]

What happens with room ownership when the owner leaves while other users are still in the room?

In our experience it seems like the room ownership may automatically change back to the server...

If this is the case it is very dangerous, because the room no longer seems to be auto-deleted once everyone leaves.

Is this what is supposed to happen?

[Lapo]

No, this doesn't happen.
However a room may not removed if the connection of the owner was not shut down properly (ghost connection).
Usually after some time a TCP connection timeout should be fired and rooms should be finally removed.

At the moment we're not able to reproduce this issue in a local network, but if you are let us know the details