The logic in KeepAliveManager.schedulePingServerTask seems to be buggy.
The condition check to schedule “periodicPingExecutorService” only if “pingFailedListeners” is not empty is wrong.
Before this condition is executed, connection.createPacketCollector() is called and packet is sent. This adds PacketCollector into the “collectors” queue of Connection. The PacketCollector is removed only when response.cancel() is executed in the run() block. This never get’s executed because of the “pingFailedListeners” existence check around the “periodicPingExecutorService.schedule(new Runnable() {” block. This causes PacketCollectors for ping being added to the connection and going out of memory.
The solution is to remove the condition and let the for loop (for (PingFailedListnere listener: paingFailedListernes)) take care of existence check (pingFailedListeners is not a null Set).
Fixed code below:
private synchronized void schedulePingServerTask() {
enableExecutorService();
stopPingServerTask();
if (pingInterval > 0) {
periodicPingTask = periodicPingExecutorService.schedule(new Runnable() {
@Override
public void run() {
Ping ping = new Ping();
PacketFilter responseFilter = new PacketIDFilter(ping.getPacketID());
final PacketCollector response = connection.createPacketCollector(responseFilter);
connection.sendPacket(ping);
// Schedule a collector for the ping reply, notify listeners if none is received.
periodicPingExecutorService.schedule(new Runnable() {
@Override
public void run() {
Packet result = response.nextResult(1);
// Stop queuing results
response.cancel();
// The actual result of the reply can be ignored since we only care if we actually got one.
if (result == null) {
for (PingFailedListener listener : pingFailedListeners) {
listener.pingFailed();
}
}
}
}, SmackConfiguration.getPacketReplyTimeout(), TimeUnit.MILLISECONDS);
}
}, getPingInterval(), TimeUnit.MILLISECONDS);
}
}