Patch - static initialization of ServiceDiscoveryManager

Smack Smack Dev
1

I’'d like to submit a patch that solves a problem I have had. ServiceDiscoveryManager has a static initialization block:

// Create a new ServiceDiscoveryManager on every established connection
static {
    XMPPConnection.addConnectionListener(new ConnectionEstablishedListener() {
        public void connectionEstablished(XMPPConnection connection) {
            new ServiceDiscoveryManager(connection);
        }
    });
}

The problem wit this is that it is not guaranteed to be run when a new connection is established. If a XMPPConnection is loaded and a new connection created before ServiceDiscoveryManager is loaded by the ClassLoader, You will have connection with no associated ServiceDiscoveryManager.

This means that,

ServiceDiscoveryManager.getInstanceFor(connection);

returns a null, and statements such as,

FileTransferManager ftMgr = new FileTransferManager(connection);

will fail with a NullPointerException for that connection.

I suggest removing this static initialization block from ServiceDiscoveryManager and adding exactly the same block to XMPPConnection. I realise this breaks encapsulation - the connection shouldn’‘t need to know about the ServiceDiscoveryManager - but I don’'t see another way of ensuring that a ServiceDiscoveryManager exists for every connection except by making it a part of XMPPConnection initialization.

MultiUserChat and XHTMLManager use the same mechanism and are probably similarly vulnerable.

There may be a better pattern to use for this that doesn’'t require XMPPConnection to know about these classes. However this is a simple fix that would solve existing bugs now.

Message was edited by: nut

2

Yes you are right, ServiceDiscoveryManager manager needs to be loaded by the ClassLoader before a creating any new connection. The point is that it actually do, is a little hard to find but this is how it works:

There’'s also a static statement in XMPPConnection which loads the SmackConfiguration

static {
    ...
    SmackConfiguration.getVersion();
}

This one also have a static statement which forces the ClassLoader to load all the classes specified in the smack-config.xml file.

static {
   ...
   parseClassToLoad(parser);
   ...
}

<!-- Classes that will be loaded when Smack starts -->
    <startupClasses>
        <className>org.jivesoftware.smackx.ServiceDiscoveryManager</className>
        <className>org.jivesoftware.smack.PrivacyListManager</className>                <className>org.jivesoftware.smackx.XHTMLManager</className>
        <className>org.jivesoftware.smackx.muc.MultiUserChat</className>
        <className>org.jivesoftware.smackx.filetransfer.FileTransferManager</className>
        <className>org.jivesoftware.smackx.LastActivityManager</className>
    </startupClasses>

If you are having problems with this it could probably be because you don’'t have the Smack configuration file in the META-INF directory.

3

Damn, it was the config file. It was getting into the build proper but not the test build Oh well, my bad.