Calling "disconnect" on XMPPTCPConnection leaks Thread (4.0.0-rc1)

The current problem is that the finalize method is never triggered.

How do you know that?

Even if I set all the references to the XMPPConnection object to null, the thread just keeps running.

Garbage collection is non-determinstic. It could take a while until it is performed. Or, and that is worse, you/we are leaking the connection instance.

I know the finalize method is not called because I’ve implemented a subclass of XMPPTCPConnection that logs a “finalize” statement as soon as “finalize” is being called. That never happens.

Moreover, I’ve created a simple test class to demonstrate the behaviour;

If you run that class (using Smack library 4.0.0-rc2 source from github) you’ll notice that the application will never stop running. I’m 100% positive that -in this test scenario- there are no more references to the “connection” object.

Wouldn’t it be okay for you to just make the executor properties in XMPPConnection (executorService and listenerExecutor) protected instead of private? Then I can subclass the connection which would immediatly solve my case without disrupting the current/future interface.

System.gc() is not sufficient for your test case as it’s not deterministic, ie. it’s only a hint to the VM that it’s now a good time for a GC run.

Wouldn’t it be okay for you to just make the executor properties in XMPPConnection (executorService and listenerExecutor) protected instead of private?
No, I still have no proof that there is actually any problem that needs to be solved and one of the worst things you can do in software development is to introduce code for problems that do not exist.

How I see it: The only case in which finalize is never called is that when there is still a non-weak reference path from a gc root to the connection instance. And if this is true, we have a total different problem, namely leaking the connection instance. And then we should debug and solve that.

That doesn’t seem to work, git.exe is in my PATH but the error persists:

E:\Documents\workspaces\eclipse-android\Smack>gradle assemble

FAILURE: Build failed with an exception.

  • Where:

Build file ‘E:\Documents\workspaces\eclipse-android\Smack\build.gradle’ line: 217

  • What went wrong:

A problem occurred evaluating root project ‘Smack’.

assert !gitCommit.isEmpty()

|| |

|“” true

false

  • Try:

Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output.

BUILD FAILED

Total time: 8.135 secs

E:\Documents\workspaces\eclipse-android\Smack>

Ok, I’ve updated the source from git, and your latest changes seem to have fixed my problem, thank you!

Thanks for reporting back

Hi Flow, so far everything is OK with 4.0.0. RC2, but i am confused about Smack Executor Service remains in DDMS when I stop the service in android. Every time I start/stop the service, a new pair of Smack Executor Service appears on the DDMS (one with status Wait and the other with status TimedWait). I am using the asmack-android-8-4.0.0-rc2-SNAPSHOT-2014-05-22.jar

Is it something normal, or i have messed up something?

That a new pair appears is definetly normal, but the old ones should eventually go away. Rember that it’s not deterministic when this will happen.

In my case, the old ones do not go away. If i start/stop the service n-times, I got n-pairs of Smack Executor Service remains in DDMS

In my case, the old ones do not go away. If i start/stop the service n-times, I got n-pairs of Smack Executor Service remains in DDMS

Like I said, it’s not deterministic and espically there is no reason why starting/stoping the service would trigger the cleanup of those threads.

I’ve just released aSmack 4.0.0-rc2-SNAPSHOT-2014-05-23 which contains a fix that should prevent the threads from leaking. Using a anonymous inner class as thread factory resulting in a reference from a gc root to this anonymous class which of course prevented the whole class of the inner class, ie the XMPPConnection instance, from being gc’ed.

Note that my statement that the cleanup of those threads is non-deterministic still holds.