I'm developing an Android 2.3.3 application with a service. I have this inside that service to communicate with Main activity:
public class UDPListenerService extends Service
{
private static final String TAG = "UDPListenerService";
//private ThreadGroup myThreads = new ThreadGroup("UDPListenerServiceWorker");
private UDPListenerThread myThread;
/**
* Handler to communicate from WorkerThread to service.
*/
private Handler mServiceHandler;
// Used to receive messages from the Activity
final Messenger inMessenger = new Messenger(new IncomingHandler());
// Use to send message to the Activity
private Messenger outMessenger;
class IncomingHandler extends Handler
{
@Override
public void handleMessage(Message msg)
{
}
}
/**
* Target we publish for clients to send messages to Incoming Handler.
*/
final Messenger mMessenger = new Messenger(new IncomingHandler());
[ ... ]
}
And here, final Messenger mMessenger = new Messenger(new IncomingHandler());
, I get the following Lint warning:
This Handler class should be static or leaks might occur: IncomingHandler
What does it mean?
If IncomingHandler
class is not static, it will have a reference to your Service
object.
Handler
objects for the same thread all share a common Looper object, which they post messages to and read from.
As messages contain target Handler
, as long as there are messages with target handler in the message queue, the handler cannot be garbage collected. If handler is not static, your Service
or Activity
cannot be garbage collected, even after being destroyed.
This may lead to memory leaks, for some time at least - as long as the messages stay int the queue. This is not much of an issue unless you post long delayed messages.
You can make IncomingHandler
static and have a WeakReference
to your service:
static class IncomingHandler extends Handler {
private final WeakReference<UDPListenerService> mService;
IncomingHandler(UDPListenerService service) {
mService = new WeakReference<UDPListenerService>(service);
}
@Override
public void handleMessage(Message msg)
{
UDPListenerService service = mService.get();
if (service != null) {
service.handleMessage(msg);
}
}
}
See this post by Romain Guy for further reference
As others have mentioned the Lint warning is because of the potential memory leak. You can avoid the Lint warning by passing a Handler.Callback
when constructing Handler
(i.e. you don't subclass Handler
and there is no Handler
non-static inner class):
Handler mIncomingHandler = new Handler(new Handler.Callback() {
@Override
public boolean handleMessage(Message msg) {
// todo
return true;
}
});
As I understand it, this will not avoid the potential memory leak. Message
objects hold a reference to the mIncomingHandler
object which holds a reference the Handler.Callback
object which holds a reference to the Service
object. As long as there are messages in the Looper
message queue, the Service
will not be GC. However, it won't be a serious issue unless you have long delay messages in the message queue.
Here is a generic example of using a weak reference and static handler class to resolve the problem (as recommended in the Lint documentation):
public class MyClass{
//static inner class doesn't hold an implicit reference to the outer class
private static class MyHandler extends Handler {
//Using a weak reference means you won't prevent garbage collection
private final WeakReference<MyClass> myClassWeakReference;
public MyHandler(MyClass myClassInstance) {
myClassWeakReference = new WeakReference<MyClass>(myClassInstance);
}
@Override
public void handleMessage(Message msg) {
MyClass myClass = myClassWeakReference.get();
if (myClass != null) {
...do work here...
}
}
}
/**
* An example getter to provide it to some external class
* or just use 'new MyHandler(this)' if you are using it internally.
* If you only use it internally you might even want it as final member:
* private final MyHandler mHandler = new MyHandler(this);
*/
public Handler getHandler() {
return new MyHandler(this);
}
}
Myclass
should be declared as public Handler getHandler()
instead of public void
This way worked well for me, keeps code clean by keeping where you handle the message in its own inner class.
The handler you wish to use
Handler mIncomingHandler = new Handler(new IncomingHandlerCallback());
The inner class
class IncomingHandlerCallback implements Handler.Callback{
@Override
public boolean handleMessage(Message message) {
// Handle message code
return true;
}
}
With the help of @Sogger's answer, I created a generic Handler:
public class MainThreadHandler<T extends MessageHandler> extends Handler {
private final WeakReference<T> mInstance;
public MainThreadHandler(T clazz) {
// Remove the following line to use the current thread.
super(Looper.getMainLooper());
mInstance = new WeakReference<>(clazz);
}
@Override
public void handleMessage(Message msg) {
T clazz = mInstance.get();
if (clazz != null) {
clazz.handleMessage(msg);
}
}
}
The interface:
public interface MessageHandler {
void handleMessage(Message msg);
}
I'm using it as follows. But I'm not 100% sure if this is leak-safe. Maybe someone could comment on this:
public class MyClass implements MessageHandler {
private static final int DO_IT_MSG = 123;
private MainThreadHandler<MyClass> mHandler = new MainThreadHandler<>(this);
private void start() {
// Do it in 5 seconds.
mHandler.sendEmptyMessageDelayed(DO_IT_MSG, 5 * 1000);
}
@Override
public void handleMessage(Message msg) {
switch (msg.what) {
case DO_IT_MSG:
doIt();
break;
}
}
...
}
I am not sure but you can try intialising handler to null in onDestroy()
I'm confused. The example I found avoids the static property entirely and uses the UI thread:
public class example extends Activity {
final int HANDLE_FIX_SCREEN = 1000;
public Handler DBthreadHandler = new Handler(Looper.getMainLooper()){
@Override
public void handleMessage(Message msg) {
int imsg;
imsg = msg.what;
if (imsg == HANDLE_FIX_SCREEN) {
doSomething();
}
}
};
}
The thing I like about this solution is there is no problem trying to mix class and method variables.
Success story sharing
get()
will return null when referenced object was gc-ed. In this case, when service is dead.