Best practice for eventbus with thread safety

My app has activities for the user interaction and a background service which is the only place where the data model is being modified. The background service listens to actions that where made by the user as well as incoming messages from the network. Therefore concurrency issues can arise which I try to prevent by using a handler. For the event layer I use greenrobots Eventbus.

This is all working well but I wonder if there is a smarter/faster/less code extensive (and therefore less error prone) way to handle this use case?

To be more specific:

Is there a way to ensure serial execution of the onEvent methods without a handler?
Is there an alternative to having onEvent methods for each possible event?
Is there a better pattern for what I am doing here?

This is my approach:

In the oncreate method I do register the service (in case of an activity I do this in onstart)

@Override
public void onCreate() {
    super.onCreate();
    ...
    EventBus.getDefault().register(this);
}

And in the onDestroy I do unregister again:

@Override
public void onDestroy() {
    super.onDestroy();
    ....
    EventBus.getDefault().unregister(this);
}

Whenever I react to an incoming event I want to ensure serial execution as there can be concurreny issues because there are incoming events from user interactions as well as from other users via networking. So I decided to work with a handler:

private Handler handler = new Handler(){
        @Override
        public void handleMessage(Message msg) {
            Object receivedEvent = msg.obj;
            if(receivedEvent instanceof EditUser)
            {
                processEditUserBroadcast((EditUser)receivedEvent);
            }           
            else if(receivedEvent instanceof JoinParty)
            {
                processJoinPartyBroadcast((JoinParty)receivedEvent);
            }
            else if(receivedEvent instanceof LeaveParty)
            {
                processLeavePartyBroadcast();
            }
            else if(receivedEvent instanceof SendMessage)
            {
                processSendMessageBroadcast((SendMessage)receivedEvent);
            }
            else if(receivedEvent instanceof ReceivedMessage)
            {
                processReceivedMessageBroadcast((ReceivedMessage)receivedEvent);
            }       
            else if(receivedEvent instanceof Reset)
            {
                processResetBroadcast();
            }
            else if(receivedEvent instanceof ImageDownloadFinished)
            {
                processImageDownloadFinishedBroadcast((ImageDownloadFinished)receivedEvent);
            }
        }
    };  
    return handler;
}

For each event of interest I do have an onEvent method which is doing nothing but passing the event to the handler to ensure serial execution via a small "passToHandler" helper function

public void passToHandler(Handler handler, Object object)
{
    Message message = handler.obtainMessage();
    message.obj = object;
    handler.sendMessage(message);
}

public void onEvent(EditUser editUser)
{
    passToHandler(handler,editUser);
}

public void onEvent(JoinParty joinParty)
{
    passToHandler(handler,joinParty);
}

public void onEvent(LeaveParty leaveParty)
{
    passToHandler(handler,leaveParty);
}

public void onEvent(SendMessage sendMessage)
{
    passToHandler(handler,sendMessage);
}

public void onEvent(ReceivedMessage receivedMessage)
{
    passToHandler(handler,receivedMessage);
}

public void onEvent(Reset reset)
{
    passToHandler(handler,reset);
}

public void onEvent(ImageDownloadFinished imageDownloadFinished)
{
    passToHandler(handler,imageDownloadFinished);
}

The "process.." methods are where the "data magic" happens and shouldn´t be relevant for my question.

And of course for each possible event I did create a class which is usually quite slim like this:

public class JoinParty {
    private String partyCode;

    public JoinParty(String partyCode) {
        super();
        this.partyCode = partyCode;
    }
    public String getPartyCode() {
        return partyCode;
    }   
}

Thank you for posting this Matthias! I think you bring up a very important point about thread safety with GreenRobot EventBus that can easily be missed by users of it.

I think you are quite possibly heading down the right path, though I'm new to GreenRobot EventBus and Android (but not Java). If I read the GreenRobot EventBus source code correctly, one other possible benefit to this approach is that post of the SendMessage event to your onEvent() method immediately returns (after calling sendMessage on the Handler) allowing the EventBus to continue posting it to any other subscribers without delay of the actual processing by your class. This may or may not be what you desire though.

With the approach that you have given, the other thing you need to ensure is that if you take an approach like this that there are no other public methods to your class that has all of your onEvent() methods and the methods such as processEditUserBroadcast(). Otherwise, while you have ensured that all of the processing of the events received from the EventBus are actually handled on a single thread (in a serial manner), some other class might call a public method of this class on a different thread and then cause you thread safety issues again.

If you know that you do need to support other public methods on this class, doing what you have done here at least gets all of the onEvent() methods handling onto a single thread (that of the Looper for the thread that creates the Looper from what I read in the doc for the Looper class) and that simplifies things at least some. You may also then need to apply some synchronization to the public methods and all of the other methods such as processEditUserBroadcast() so as to guarantee safe access to the data members of the class from multiple threads if you are going to have other public methods on this class. Alternatively, depending on what those data members are and what your needs are, you might be able to get by with simply making some of them volatile, atomic, or using the concurrent collections, etc. It all depends on what the read and write access needs are and also the needed granularity of those accesses.

Does this help at all? For those that are well versed with Android, Loopers, Handlers, GreenRobot EventBus, etc. have I misspoken at all?

Recommended topics

Hot tags