I am looking for some advice in my little smack project. I made my own IM-App on Android. For this reason I want to close my connections sometimes, for example if I dont have internet connection (that happens quite often) on my phone or when I don’t want to chat. In this case I call connection.disconnect(). After that I set my roster null, my connection referece null and let my GC do its work.
Unfortunatly is the disconnect method not closing all the smack threads related to the closed connection. In detail: By establishing a connection, 4 threads are started:
Smack Packet Writer (0)
Smack Packet Reader (0)
Smack Executor Service 0 (0)
Smack Scheduled Ping Executor Service (0)
But after disconnecting only 2 threads are being closed (the reader and the writer). So two threads are still running (the two executors) even though there is no connection or a connection object. This causes 6 threads after the next connect() call:
Smack Executor Service 0 (0)
Smack Scheduled Ping Executor Service (0)
Smack Packet Writer (1)
Smack Packet Reader (1)
Smack Executor Service 0 (1)
Smack Scheduled Ping Executor Service (1)
Any idea what I did wrong or is this a (maybe known) bug?
Those executors are shut down in the finalize() method of the connection and PingManager. Now before anyone jumps in with “But finalizers are bad, you shouldn’t use them”, then rest assured that I’m well aware of the problematic of finalizers but I believe that this is actually a situation where they are useful (of course I’d be happy to be proven wrong or presented with a “better” design, feel free to join #smack to discuss this).
I think I don’t have to explain that before the finalizer will be called, the connection instanced must become eligible for garbage collection. This should be the case when you drop the last reference to the instance. If there is still a strong reference path from a gc root to the instance, which can be determined/proved by inspecting a heap dump, then Smack has a memory leak which I’m happy to fix. But I’ve no indications that this is in-fact the case.
Also note that alpha6 is the latest alpha of Smack 4.1. I recommend using that version.
Maybe another remark: It’s always a good idea to reuse the connection instance as long as possible. Especially on resource constraint platforms like Android.
Using 4.1.0-alpha3, we had a similar issue where the number of threads named “Smack Scheduled Ping Executor Service” kept on increasing at every connection we opened and then closed. We wanted to move to 4.1.3 to try to fix this issue, but a reported GCM-related bug in 4.1.3 prevented us to do so. So we decided to implement a crude way to get rid of all these “Smack Scheduled Ping Executor Service” threads when closing the connection. It involves playing with private static data held in the PingManager class, using reflection.
Our connection closing logic now looks like this:
import java.lang.reflect.Field;
[…]
try {
connection.removePacketListener(packetListener);
} catch (Exception e1) {
}
if (connection.isConnected())
try {
connection.disconnect();
} catch (Exception e2) {
}
try { final Field f_INSTANCES = PingManager.class.getDeclaredField("INSTANCES"); f_INSTANCES.setAccessible(true); @SuppressWarnings("unchecked") final Map<XMPPConnection, PingManager> INSTANCES = (Map<XMPPConnection, PingManager>) f_INSTANCES.get(null); final PingManager pingManager = INSTANCES.remove(connection); if (pingManager != null) { final Field f_executorService = PingManager.class.getDeclaredField("executorService"); f_executorService.setAccessible(true); final ScheduledExecutorService executorService = (ScheduledExecutorService) f_executorService.get(pingManager); executorService.shutdown(); } } catch (Exception e3) { throw new RuntimeException("Reflection error.", e3); }
This quick and dirty fix now prevents our app from crashing due to too many threads being created by Smack. Our app was crashing after opening and closing 4800 connections. The JVM dump was showing 4800 hung Smack Scheduled Ping Executor Service threads.
We wanted to move to 4.1.3 to try to fix this issue, but a reported GCM-related bug in 4.1.3 prevented us to do so.
Which bug are you referring to? As far as I know, there is only one GCM related bug in 4.1.3 which is about the JsonExtensionProvider, but this should not stop you from using 4.1.3: You can simply replace the JsonExtensionProvider with a fixed version.