Room Creation Threads Overlapping

[ktamlyn]

There is a rare situation where two rooms with the same name are getting created with my Zone extension but when it does, it creates confusion.

There are groups of people that get invitations to join a "session". The first person to login triggers the creation of a room just for this group. If two users login within a second of eachother sometimes two rooms are created. How do I avoid this kind of problem easily?

I have conceived a sort of "request to create room" queue that waits 5 seconds to create the room. Is this the best kind of solution?

[Lapo]

You can synchronize the code that creates the Room.

[ktamlyn]

Thanks for the help. Synchronizing the code didn't seem to do the trick because I was looking for the rooms existence as my test on whether a new one should be created. Since room creation seems to work asynchronously, I had to track that if the room creation process was started withing a ConcurrentHashMap.

[Lapo]

You said you are creating the Room from server side, right?
If so, the Room is created synchronously, therefore you can apply proper synchronization to the block of code that requires serial execution.

[ktamlyn]

This is distinctly not what I am experiencing. RoomVariables are not available about 1/3 of the time when I synchronize the code. So, Synchronizing improved things, but didn't make them perfect. In the end I Synchronized the code so that I could detect if a room was in the process of being created. On the rare occasion a room is being created by some other user's login attempt but it hasn't been created yet, a response allows user #2 to attempt again in 1/2 second.

[Lapo]

I need to see the code that is used, to understand what you mean.

Thanks

[ktamlyn]

What I'm doing doesn't feel right. Here are the two relevant handlers. Users login and are sent to the lobby. Once there they poll the extension to find the proper "sessionId". With a chosen sessionId the extension knows which room to place the user into, but if that room doesn't exist yet, create it. The problem is occuring if I perform "auto-logins" of two different users going to the same session at the exact same time. The end result sometimes is the creation of two rooms with the same name. When user 2 comes along the room doesn't exist yet, but it has already been requested to be created. The second user requests a room to be created, and we end up with two rooms with the same name.

Code: Select all

public class EnterSessionRoomRequestHandler extends BaseClientRequestHandler {

    public ZoneExtension parentEx;

    @Override
    public void handleClientRequest(User user, ISFSObject params) {

        synchronized (this) {

            parentEx = (ZoneExtension) getParentExtension();
            Integer sessionId = params.getInt("sessionId");

            trace("EnterSessionRoomRequestHandler()");

            // validate user/session pair (again, just in case of clientside bug)
            try {
                parentEx.isSessionIdValid(user.getName(), sessionId);
            } catch (SFSExtensionException e) {
                e.printStackTrace();
            }

            UserVariable userVariable =  new SFSRoomVariable("sessionId", sessionId);
            userVariable.setHidden(true);
            try {
                user.setVariable(userVariable);
            } catch (SFSVariableException e) {
                e.printStackTrace();
            }

            Room room = parentEx.getParentZone().getRoomByName("room"+sessionId.toString());
            if(parentEx.createdRooms.containsKey("room"+sessionId.toString())) {
                // room is not ready, so send user a message to try again shortly
                parentEx.send(ZoneExtension.SFSEX_REQUEST_ENTER_SESSION_ROOM, null, user);
            } else if(room == null) {
                parentEx.createdRooms.put("room"+sessionId.toString(), 1);
                CreateRoomSettings roomSettings = new CreateRoomSettings();
                roomSettings.setName("room"+sessionId.toString());

                RoomVariable sessionIdVariable = new SFSRoomVariable("sessionId", sessionId);
                sessionIdVariable.setHidden(false);

                // TODO: make debugging/testing dynamic
                RoomVariable debugVariable = new SFSRoomVariable("debug", true);
                debugVariable.setHidden(false);

                List<RoomVariable> roomVariables = new ArrayList<RoomVariable>();
                roomVariables.add(sessionIdVariable);
                roomVariables.add(debugVariable);
                roomSettings.setRoomVariables(roomVariables);

                CreateRoomSettings.RoomExtensionSettings roomExtensionSettings = new CreateRoomSettings.RoomExtensionSettings("customRoom", "roomExtension.RoomExtension");
                roomSettings.setMaxVariablesAllowed(12);
                roomSettings.setExtension(roomExtensionSettings);

                parentEx.trace("****   user to create room: " + user.getName() + "     ****");

                try {
                    getApi().leaveRoom(user, parentEx.getParentZone().getRoomByName("lobby"), false, false);
                    getApi().createRoom(parentEx.getParentZone(), roomSettings, user, true, null, true, true);
                } catch (SFSCreateRoomException e) {
                    e.printStackTrace();
                }

            } else if (room!=null) {
                try {
                    getApi().leaveRoom(user, parentEx.getParentZone().getRoomByName("lobby"), false, false);
                    getApi().joinRoom(user, room, null, false, null, true, true);
                } catch (SFSJoinRoomException e) {
                    e.printStackTrace();
                }
            }
            trace("EnterSessionRoomRequestHandler(): Finish");
        }

    }


}

Code: Select all

public class RoomAddedEventHandler extends BaseServerEventHandler {

    @Override
    public void handleServerEvent(ISFSEvent event) throws SFSException
    {
        synchronized (this) {
            ZoneExtension parentEx = (ZoneExtension) getParentExtension();
            Room room = (Room) event.getParameter(SFSEventParam.ROOM);

            if(parentEx.createdRooms.containsKey(room.getName())) parentEx.createdRooms.remove(room.getName());

            List<User> users = parentEx.getParentZone().getRoomByName("lobby").getUserList();
            for(User user:users) {
                if(user.getVariable("sessionId").getIntValue()==room.getVariable("sessionId").getIntValue()) {
                    try {
                        getApi().leaveRoom(user, parentEx.getParentZone().getRoomByName("lobby"), false, false);
                        getApi().joinRoom(user, room, null, false, null, true, true);
                    } catch (SFSJoinRoomException e) {
                        e.printStackTrace();
                    }
                    break;
                }
            }
        }
    }


}

[Lapo]

The first thing I need to ask is ... do you need the two handlers to be synchronized among them?
Or do you just need that they are separately synchronized to the possible multi threads that will invoke them?

[Lapo]

Another problem I see is that you are setting both User and Room Variables in the wrong way.
By calling User.setVariable(...) or Room.setVariables(...) you are just changing the settings locally but not dispatching any updates to client.
While this may be ok for hidden variables (server side only) it is not for regular variables and in your code you do have some.

The proper way is to use the method calls on the SFSApi object, which is available in any Extension or Handler by simply calling getApi()
http://docs2x.smartfoxserver.com/api-do ... FSApi.html

[ktamlyn]

Synchronizing the code separately is what I understand I am doing. The room variables and user variables are not needed on the client side. I'm passing data to the new room via a room variable. Is there a preferred method?

Those red herrings aside, my problem is that when two users make a request to join a session simultaneously before a session room has been created, sometimes two rooms are created with the same name. This is explicitly my problem. It makes sense that synchronizing the code that creates the new room should have fixed the problem, but it didn't. I had to track attempts to create the room in a variable (the ConcurrentHashMap mentioned above) to attempt to not make two separate room creation calls, even then, sometimes my code leads to two rooms with the same name.

Here is the situation:

1) users are invited to scheduled sessions that are exclusive to that person + a small group of other participants
2) these sessions can be logged into early
3) each session is its own room run by a separate room extension

I have chosen to dynamically create a room for the session when ever *any* user logs in because everyone/anyone should have access to the initial Welcome screen information immediately. It doesn't make sense to do this any other way.

[ktamlyn]

P.S. the "proper way" to create user variables via getAPI() vs "improper" is a trap that appears to catch many newbies to SFS. It got me, wasted at least 3 hours of my time before I discovered that the getApi method specifically addressed whether clients received events. It should be in the documentation everywhere it needs to be when events are or are not fired. I find myself spending time performing test cases to figure out somethings that should be in the api documentation.

[Lapo]

Those red herrings aside, my problem is that when two users make a request to join a session simultaneously before a session room has been created, sometimes two rooms are created with the same name. This is explicitly my problem. It makes sense that synchronizing the code that creates the new room should have fixed the problem, but it didn't. I had to track attempts to create the room in a variable (the ConcurrentHashMap mentioned above) to attempt to not make two separate room creation calls, even then, sometimes my code leads to two rooms with the same name.

Yes, I noticed that you have added what looks like an extra layer of room management which seems superfluous.

Here is the situation:

1) users are invited to scheduled sessions that are exclusive to that person + a small group of other participants
2) these sessions can be logged into early
3) each session is its own room run by a separate room extension

I have chosen to dynamically create a room for the session when ever *any* user logs in because everyone/anyone should have access to the initial Welcome screen information immediately. It doesn't make sense to do this any other way.[/quote]

I have some problems with the above terminology you are using because it collides with fundamental concepts in SmartFoxServer.
Session = a client connection in SFS2X, the Session is not just the mere socket connection but a more complex object/concept
Log in = a Session can login into a Zones/application and become a User in the system. The User has permissions that allow him to send Requests.

Terminology aside, and hoping that this doesn't get in the way, I can't see how two Rooms can be created with the same name if the synchronization is done correctly.
Do you allow clients to create Rooms from the client side too? If so your Extension and the client Requests will run concurrently, thus defeating the synchronization.
It is the only way in which I think you could run into problems.

Thanks

[ktamlyn]

The code that creates rooms is exclusive to the code you see here (clients don't make room creation requests). I apologize for the confusing terminology. A "session" is a term used in the product I'm developing. In my previous post I am not using both meanings, just the one for my application, so you can swap the word "session" with "activity" or "game" in every case. I will try to keep an eye on this and see if I can provide more information as I continue to develop.

Its possible that issue is specific to my development environment at this point (since I reduced the likelihood of duplicate rooms from 50% to about 10% bu synchronizing the code). I am opening two tabs simultaneously in Chrome so the attempts to connect to SFS are nearly exactly the same time.

[Lapo]

Oh wait, it dawned on me what the problem is.
You use a synchronized(this) statement, the problem is that this changes on every request unless you declare otherwise.

You can choose if Handlers are created once or on every request, depending if you keep state or not. By default it's the latter but you can change this via an annotation. Add this line on top of your class definition and that's it.

Code: Select all

@Instantiation(SINGLE_INSTANCE)

This way the synchronized will work correctly.
More on annotations here:
http://docs2x.smartfoxserver.com/Advanc ... extensions
see the "Advanced Extension features" section

[ktamlyn]

Thank you, but when I placed @Instantiation(SINGLE_INSTANCE) above the class declaration, I got "cannot resolve class". From another post I found out that I had to place

Code: Select all

@Instantiation(Instantiation.InstantiationMode.SINGLE_INSTANCE)