Concurrency issues

Openfire Plugins Kraken - IM Gateway
1

Hi Daniel,

I found two concurrency issues in the IM Gateway plugin.

The first is quite easy to fix. In the SessionManager class a HashMap is used to store sessions. The SessionReaper periodically goes over the values in this map. However, if a change to the map (e.g. a new session is created) is made when the SessionReaper is iterating through the values a ConcurrentModificationException will be thrown causing the Timer thread to die. This can easily be solved by changing the type of the map to ConcurrentHashMap.

The second issue will be a bit harder to fix. When two resources of a user come online at the same time, two sessions are created, but only one is kept in the session map mentioned above (the first is overwritten). This can cause very strange behaviour, like people staying logged in to a network while they are no longer online, receiving “you have been disconnected from msn” messages whie you actually still remain logged in, people chatting to you, but you cannot chat back etc.

I think creating sessions should be synchronized in some way, preferrably per XMPP user. I have created a workaround which achieves this, but I would prefer not to share it here, because it violates some rules of good programming practice ;). In order to keep the impact of this synchronization as low as possible all initialization done for a session class should probably be moved out of the constuctor.

Hope this helps.

Regards,

Lars

2

Wow, thanks lars! GATE-253 and GATE-254!

I hadn’‘t thought about GATE-254. I think I have some ideas on how to take care of that. Hopefully it won’'t be too hard to duplicate. Are you able to duplicate it easily? I imagine that if I had Psi configured for two connections, ''cept they were the same account with different resources, that I would most likely trigger this bug.

Either way, if you wouldn’‘t mind, I would like to see what your solution is because I’‘m always open for looking at other ways of thinking through things! If you don’‘t want to share here, would you mind emailing to me? (Since you dislike it, I’‘m not going to implement it that way ;D but like I said, if it’‘s easy to share, I’'d love to hear it regardless!)

3

Hi Daniel,

Actually the second problem is very easy to duplicate. I created a simple test using Smack:

import org.jivesoftware.smack.XMPPConnection;
import org.jivesoftware.smack.XMPPException;
import org.jivesoftware.smack.util.StringUtils; public class LoginTest {      private static final String domain = "CHANGE_THIS";
     private static final String user = "CHANGE_THIS";
     private static final String password = "CHANGE_THIS";      private XMPPConnection con;
     
     private static LoginTest client1;
     private static LoginTest client2;
     
     public static void main(String [] args) {
          XMPPConnection.DEBUG_ENABLED = true;
          client1 = new LoginTest();
          client2 = new LoginTest();
          try {
               new Thread() {
                    @Override
                    public void run() {
                         try {
                              client1.connect();
                         } catch (XMPPException e) {}
                    }
               }.start();
               client2.connect();
               try {
                    Thread.sleep(300000);
               } catch (InterruptedException e) {}
          } catch (XMPPException e) {
               e.printStackTrace();
          } finally {
               client1.disconnect();
               client2.disconnect();
          }
     }      private void connect() throws XMPPException {
          con = new XMPPConnection(domain);
          con.connect();
          con.login(user, password, StringUtils.randomString(10));
     }      private void disconnect() {
          if (con != null) {
               con.disconnect();
          }
     }
}

Make sure the user that is used in the test is registered with e.g. the MSN transport and check the debug log to verify that two sessions are created.

I will let you know the solution I applied to our code.

Regards, Lars