XMPPConnection memory leak

Hi all!

I’m facing memory issues with Smack API version 3.0.4:

About

10% of my XMPPConnections (and related objects) aren’t

garbage-collected after disconnecting and setting the reference to null; not even by a forced collection.

I’ve

done some extensive testing using a memory profiler, which shows that

all of my objects (which open and disconnect the XMPPConnection) are

propperly garbage-collected - it’s only org.jivesoftware objects

remaining in memory. I also made absolutely sure that

connection.disconnect() will be called on all connections before

releaseing them.

According to the memory-profiler, the

XMPPConnection objects remaining in memory are still referenced from

PrivacyListManager, ServiceDiscoveryManager and MultiUserChat$1$1 (?)

objects.

It seems that the Java garbage collection (java version

1.5.0_09) is having problems with collection large sets of objects

containting circular references.

Maybe setting some of these references to null in XMPPConnection.disconnect() would help?

Thanks for any feedback!

Florian Kirchmeir

PS: Objects still referenced by the XMPPConnection are:

socket, accountManager, chatManager, configuration and saslAuthentication (and some String values).

At least the accountManager, chatManager and saslAuthentication objects contain back-references to the containing XMPPConnection object.

Might be worthwhile to release them on a disconnect() ?

Best regards,

Florian Kirchmeir

Hi all!

I think I have found the problem in the Smack source:

Both PrivacyListManager and ServiceDiscoveryManager are holding a static Map of instances, using the XMPPConnection object as map key. New instances register a ConnectionListener with the connection, using the connectionClosed() callback to remove the instance (and the XMPPConnection object) from the static Map when the connection closes normally.

However, if the connection is closed on error, the call to the connectionClosedOnError() callback is ignored and the XMPPConnection object along with the associated Manager instance will never be removed from the static Map, and thus cannot be garbage collected.

A similar problem is found in the MultiUserChat class, that also ignores connectionClosedOnError() to do propper cleanup.

The attached patch seems to resolve the problem for me.

Any comments?

Best regards,

Florian Kirchmeir
connectionClosedOnError.patch (1926 Bytes)

Hi!

Any statement regarding this from “official” side?

Any chance my patch will find it’s way into the next release?

I’d like to open a JIRA ticket reagarding this, but JIRA access seams to be read-only, even after logging in?!

Thanks for any feedback!

Florian Kirchmeir

I think I’m having this issue. A long running Java application server with a MultiUserChat.

The closest JIRA I can find is: http://www.igniterealtime.org/issues/browse/SMACK-129

Hi!

Was this issue resolved in the 3.1.0 release? My 3.1.0 MUC client continues to throw Out of Memory errors and was wondering if they had applied a patch but this was something else?

Thanks

I checked the patch when looking for community patches (see http://www.igniterealtime.org/community/docs/DOC-1722), but found some problems. Essentially Florian is right with his patch that keeping the connection in static hash maps will cause a memory leak. Unfortunately his patch breaks reconnection. What I think needs to be done whenever a static hashmap is used is this:

public void connectionClosedOnError(Exception e) {
    // Unregister this instance since the connection has been closed
    instances.remove(connection);
} public void reconnectionSuccessful() {
    // Register this instance since the connection has been
    // reestablished
    instances.put(connection, AdHocCommandManager.this);
}

As I think only is done correctly in the AdHocCommandManager.

Can anybody verify this?

Christopher