A subtle and interesting race condition

Race conditions are an important and common topic when discussing multithreaded programming. Every instructor benefits by including realistic examples when lecturing on this subject. The best source of realistic examples is, arguably, real examples from real code. In the spirit of recording such an example both for my benefit and for that of anyone reading this, let me describe for you a race condition I fixed today.

The program is an Android app that sends commands to an Arduino board to control a robot. When the user is ready to begin controlling the robot, it starts up a thread to handle the communication. The communication thread maintains a TreeSet data structure (trueFlags) that tracks which sensory conditions are true. On each iteration, it updates the user interface to display these conditions for the user.

In an Android app, to update the user interface from a separate thread requires specifying a block of code for the UI to run when it is ready. In our example, the code looks like this:

    private void updateGUI(final Action currentAction) {
        getActivity().runOnUiThread(new Runnable() {   
            @Override
            public void run() {
                allTrueFlags.setText(trueFlags.toString());

                StringBuilder values = new StringBuilder();
                for (int i : sensorValues) {
                    values.append(i + ", ");
                }
                bytesReceived.setText(values.toString());
                bytesSent.setText(currentAction.toString());
            }
        });
    }

It looks pretty harmless. After all, only the UI data structures are being modified, and only on a thread set aside for that purpose! But it caused our app to crash at seemingly random intervals as the robot wandered around a room.

The trouble was the call to trueFlags.toString(). This TreeSet is constantly being modified in the communication thread. On the rare occasions when these modifications were interrupted to run the UI thread, a ConcurrentModificationException was thrown when trying to construct a string from it. Often, the robot would run for minutes at a time before this occurred!

It was not difficult to fix this bug, but it took quite a lot of effort to find it! Hopefully, posting this example will help someone else out there identify a similar bug sometime.  Here is the fixed code that avoids the bug, by moving the string conversion back into the communication thread:

    private void updateGUI(final String trueFlagString, final Action currentAction) {
        getActivity().runOnUiThread(new Runnable() {
            @Override
            public void run() {
                allTrueFlags.setText(trueFlagString);

                StringBuilder values = new StringBuilder();
                for (int i : sensorValues) {
                    values.append(i + ", ");
                }
                bytesReceived.setText(values.toString());
                bytesSent.setText(currentAction.toString());
            }
        });
    }