Memory leak in ping task

Hi,

it seems like there is a memory leak in the KeepAliveManager. Our application runs out of memory after some time. Some analysis show that this is probably caused by the ping task created in KeepAliveManager which creates a PacketCollector instance for every ping. These are added to the list of collectors and cannot be garbage collected.

Best regards,

Dominik

Thanks for the report, it is logged in Jira as SMACK-441.

You should be able to work around this by simply registering a PingFailedListener with the KeepAliveManager. It looks like the packet collector is always being created, but only closed when there is a listener registered.

I would suggest a simple no op listener as a workaround.

Here is a proposed patch:

Index: src/main/java/org/jivesoftware/smack/keepalive/KeepAliveManager.java

IDEA additional info:

Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP

<+>UTF-8

===================================================================

— src/main/java/org/jivesoftware/smack/keepalive/KeepAliveManager.java (revision 2432)

+++ src/main/java/org/jivesoftware/smack/keepalive/KeepAliveManager.java (revision )

@@ -261,11 +261,9 @@

@Override

public void run() {

Ping ping = new Ping();

  •                if (!pingFailedListeners.isEmpty()) {
    
  •                PacketFilter responseFilter = new PacketIDFilter(ping.getPacketID());
    
  •                final PacketCollector response = connection.createPacketCollector(responseFilter);
    
  •                    PacketFilter responseFilter = new PacketIDFilter(ping.getPacketID());
    
  •                    final PacketCollector response = connection.createPacketCollector(responseFilter);
    
  •                connection.sendPacket(ping);
    
  •                if (!pingFailedListeners.isEmpty()) {
    

// Schedule a collector for the ping reply, notify listeners if none is received.

periodicPingExecutorService.schedule(new Runnable() {

@Override

@@ -284,6 +282,8 @@

}

}, SmackConfiguration.getPacketReplyTimeout(), TimeUnit.MILLISECONDS);

}

  •                connection.sendPacket(ping);
    

}

}, getPingInterval(), TimeUnit.MILLISECONDS);

}

Thank you, could you attach it as unified diff file (i.e. ‘.patch’) to your post.

To attach files press on undefined word in the upper right corner of post editor (should be saying “Advanced editor”…).

Attached is the same patch.

Thanks for the pointer on the “undefined” link
o_Fix_SMACK-441.patch.zip (717 Bytes)

This has already been fixed.

I also confirmed that the proposed workaround does in fact address the problem for those using version 3.3 until 3.3.1 is released.

Hi Ron,

I am in need of a BOSH-XMPP Java library as well. Smack was the obvious front runner, but lacks the BOSH support in the official release.

Can you please attached the source code zip and the jars(with dependencies) that you are using?

Thanks a lot.

Can you explain me, why there is no fixed release availble? This bug is known since May. It renders this whole library completly useless if you don’t know about it and there is absolutly no way to find this out. It took me quite some time until I located the source of my memory leak.

Is this intended as a serious xmpp library? Then you should probably note when a library has such a serios issues. Even if you don’t want to release a fixed version (for whatever reason) than you should at least put it in red writing directly below the download button.

I’m a bit pissed rigth now. Yeah, this is a free open source library, but this does not justify why there is absolute nothing that indicates that this library will crash nearly everthing it runs in.