Thread safety issue with CommonsWare 5.9 Event Buses example

Question

I have a thread safety issue/question regarding the CommonsWare 5.9 Event Buses example.

It looks to me like there is a problem with the way the model ArrayList is accessed from multiple threads. If this works, I'd appreciate understanding why.

This code performs the declaration, initialization and populating of model.

private static final String[] items= { "lorem", "ipsum", "dolor",
  "sit", "amet", "consectetuer", "adipiscing", "elit", "morbi",
  "vel", "ligula", "vitae", "arcu", "aliquet", "mollis", "etiam",
  "vel", "erat", "placerat", "ante", "porttitor", "sodales",
  "pellentesque", "augue", "purus" };
private ArrayList<String> model=new ArrayList<String>();
private boolean isStarted=false;

@Override
public void onCreate(Bundle savedInstanceState) {
  super.onCreate(savedInstanceState);

  setRetainInstance(true);

  if (!isStarted) {
    isStarted=true;
    new LoadWordsThread().start();
  }
}

public ArrayList<String> getModel() {
  return(model);
}

class LoadWordsThread extends Thread {
  @Override
  public void run() {
    for (String item : items) {
      if (!isInterrupted()) {
        model.add(item);
        EventBus.getDefault().post(new WordReadyEvent());
        SystemClock.sleep(400);
      }
    }
  }
}

It wakes up every 400 mSec and adds an entry to model. The consumption of model is a little more complicated in that I don't readily have the code handy to post. It's down somewhere inside an ArrayAdapter. The consumption of the model is triggered by a call to ArrayAdapter.notifyDataSetChanged().

The issue/question I have is that model is written to in one thread (not the Android UI thread), but the consumption is happening on the UI thread. All of the elements of the model ArrayList are immutable, which helps, but it still doesn't seem right to me.

If this code is in fact correct, I'd like to understand why. Thanks, Lee.

Update:

I don't think my problem is with the event bus part. I think I get that. It's almost more of a java issue I have in that model is written to in one thread (worker thread, code above) while theoretically it could be simultaneously read from in another thread (UI thread). It seems to me that there needs to be some sort of concurrent access control in place on model. A synchronized block or concurrent ArrayList. But the ultimate consumer of the model is code that I don't have control over in my app, its down in ArrayAdapter somewhere. I can't add a synchronize to it. Once the ArrayAdapter is initialized with a model, the ArrayAdapterowns it and will access it from the UI thread. I'm not sure that a non-UI thread can modify model in this way.

Answer 1

You are correct -- this sample app is flawed. I was trying to aggressively minimize the code, and I went too far.

The two changes that are needed are:

1. AsyncDemoFragment should be working off of a copy of the model ArrayList<String>, initialized from the then-current model, rather than actually being the model

2. I should be supplying the word in WordReadyEvent, to allow the AsyncDemoFragment to update its copy of the model, in addition to firing notifyDataSetChanged()

This will allow the background thread to update and maintain the model master copy on the background thread, letting the UI thread know about changes as needed, and allowing the UI thread to retrieve a copy of that model for UI use.

In other scenarios, it's not out of the question to have a shared, synchronized model. That's impractical in this case, short of kcoppock's suggestion of creating a customized adapter class, which would be like swatting a fly with a Buick in this case.

I will work to fix this for the next book update.

BTW, the reason I asked whether you had read the chapter is simply because some people ask questions based solely on examining the repo, and I wanted to know if you had read the material that accompanied the sample. My comment could have been interpreted in suggesting that if you read the chapter you would have an answer to your question, and I definitely did not mean that. I apologize if my lousy phrasing caused you any problems.

Thanks for pointing out the issue!