java.util.ConcurrentModificationException -- bug when performing IO operations (not with a list)

Question

Ok. Here is the scenario. I have a form that the user fills out to create a Match object. I use an IntentService to write the information to file on a background thread. If a boolean of "true" is passed in the intent, then a corresponding ScoreFile is written too. GlobalMatch is a singleton object Here is the IntentService code:

 public class WriteMatchService extends IntentService {

        private static final String EXTRA_SCORES_FILE = "scores_file";
        public static final String REFRESH_MATCH_LIST_INTENT_FILTER = "refresh_match_list";

        public static Intent getIntent(Context context, boolean writeScoresFile) {
            Intent intent = new Intent(context.getApplicationContext(),
                    WriteMatchService.class);
            intent.putExtra(EXTRA_SCORES_FILE, writeScoresFile);
            return intent;
        }

        public WriteMatchService() {
            super("WriteMatchService");
        }

        @Override
        protected void onHandleIntent(@Nullable Intent intent) {

            boolean writeScoresFile = false;

            if (intent != null) {
                if (intent.hasExtra(EXTRA_SCORES_FILE)) {
                    writeScoresFile = intent.getBooleanExtra(EXTRA_SCORES_FILE, false);
                }
            } else {
                // this should never happen
                return;
            }

            Context context = getApplicationContext();
            // globalMatch is a singleton object
            GlobalMatch globalMatch = GlobalMatch.getInstance(context);
            Match match = globalMatch.getCurrentMatch();

            if (writeScoresFile) {
                FileUtils.writeScoresFile(context, new ScoresFile(match.getMatchId()));
            }

            FileUtils.writeMatchToFile(context, match);

            // notify the match list to reload its contents if necessary
            Intent messageIntent = new Intent(REFRESH_MATCH_LIST_INTENT_FILTER);
            LocalBroadcastManager manager = LocalBroadcastManager.getInstance(getApplicationContext());
            manager.sendBroadcast(messageIntent);

        }
    }

Here are the two methods for writing a Match file and a Scores file:

Match File:

public static void writeMatchToFile(Context context, Match match) {

        File file = new File(context.getFilesDir(), Match.getFileName(match.getMatchId().toString()));
        String jsonString = "";
        FileOutputStream fos = null;
        jsonString = new Gson().toJson(match);

        try {
            fos = context.openFileOutput(file.getName(), Context.MODE_PRIVATE);
            fos.write(jsonString.getBytes());
        } catch (IOException e) {
            e.printStackTrace();
        } finally {
            try {
                if (fos != null) {
                    fos.close();
                }
            } catch (IOException e) {
                e.printStackTrace();
            }
        }

        // after creating match, make it the current one
        SharedPreferences.Editor editor = context.getSharedPreferences(MY_GLOBAL_MATCH,
                Context.MODE_PRIVATE).edit();
        editor.putString(CURRENT_MATCH, jsonString);
        editor.apply();
    }

Match objects contain a list of Stage objects.

Here is the Match Class:

public class Match implements Parcelable {

    private static final String TAG = "Match";

    private UUID mMatchId;
    private String mClubId;
    private String mMatchName;
    private Date mMatchDate;
    private MatchLevel mMatchLevel;
    private List<Stage> mStages;
    private List<Competitor> mCompetitors;
    private String mPassword;
    private MatchType mMatchType;

    public enum MatchType {
        USPSA("USPSA"), ACTION_STEEL("Action Steel");

        private String name;

        MatchType(String name) {
            this.name = name;
        }

        @Override
        public String toString() {
            return name;
        }
    }

    private enum MatchLevel {
        I("I"), II("II"), III("III"), IV("IV"), V("V");

        private String value;

        MatchLevel(String value){
            this.value = value;
        }

        @Override
        public String toString() {
            return value;
        }
    }

    // no arg constructor, most likely to be used as initial match created upon installation
    public Match() {

        Calendar calendar = Calendar.getInstance();
        calendar.setTime(new Date());
        mMatchDate = calendar.getTime();
        mMatchType = MatchType.USPSA;
        mMatchId = UUID.randomUUID();
        mMatchName = "";
        mMatchLevel = MatchLevel.I;
        mClubId = "";
        mStages = new ArrayList<>();
        mCompetitors = new ArrayList<>();
        mPassword = "";
    }

    public Match(String matchName, Date matchDate, MatchLevel matchLevel, MatchType matchType, String clubID, String password) {

        mMatchId = UUID.randomUUID();
        mClubId = clubID;
        mMatchName = matchName;
        mMatchDate = matchDate;
        mMatchLevel = matchLevel;
        mMatchType = matchType;
        mStages = new ArrayList<>();
        mCompetitors = new ArrayList<>();
        mPassword = password;
    }

    public String getPassword() {
        return mPassword;
    }

    public void setPassword(String password) {
        mPassword = password;
    }

    public UUID getMatchId() {
        return mMatchId;
    }

    public String getClubId() {
        return mClubId;
    }

    public void setClubId(String clubId) {
        mClubId = clubId;
    }

    public String getMatchName() {
        return mMatchName;
    }

    public void setMatchName(String matchName) {
        mMatchName = matchName;
    }

    public Date getMatchDate() {
        return mMatchDate;
    }

    public void setMatchDate(Date matchDate) {
        mMatchDate = matchDate;
    }

    public MatchLevel getMatchLevel() {
        return mMatchLevel;
    }

    public String getMatchLevelString() {
        return mMatchLevel.toString();
    }

    public void setMatchLevel(MatchLevel matchLevel) {
        mMatchLevel = matchLevel;
    }

    public void setMatchLevel(String str){
        switch (str){
            case "I":
                mMatchLevel = MatchLevel.I;
                break;
            case "II":
                mMatchLevel = MatchLevel.II;
                break;
            case "III":
                mMatchLevel = MatchLevel.III;
                break;
            case "IV":
                mMatchLevel = MatchLevel.IV;
                break;
            case "V":
                mMatchLevel = MatchLevel.V;
                break;
            default:
                Log.d(TAG, "Something went wrong");
        }
    }

    public MatchType getMatchType() {
        return mMatchType;
    }

    public static MatchType getMatchTypeFromString(String matchType){
        switch (matchType){
            case "USPSA":
                return MatchType.USPSA;
            case "Action Steel":
                return MatchType.ACTION_STEEL;
            default:
                return null;
        }
    }

    public String getMatchTypeString(){
        return mMatchType.toString();
    }

    public void setMatchType(MatchType matchType){ mMatchType = matchType;}

    public void setMatchType(String matchType) {
        switch (matchType){
            case "USPSA":
                mMatchType = MatchType.USPSA;
                break;
            case "Action Steel":
                mMatchType = MatchType.ACTION_STEEL;
                break;
            default:
                Log.d(TAG, "Something went wrong");
        }
    }

    public void addStage(Stage stage) {
        mStages.add(stage);
    }

    public List<Stage> getStages() {
        return mStages;
    }

    public void setStages(List<Stage> stages) {
        mStages = stages;
    }

    public List<Competitor> getCompetitors() {
        return mCompetitors;
    }

    public void setCompetitors(List<Competitor> competitors) {
        mCompetitors = competitors;
    }

    public void addCompetitor(Competitor competitor) {
        // if adding a competitor here, assign the competitor a shooter number
        competitor.setShooterNum(mCompetitors.size() + 1);
        mCompetitors.add(competitor);
    }

    public void updateCompetitorInMatch(Competitor comp) {

        for (Competitor c : mCompetitors) {
            // this works because both will have the same ID
            if (c.equals(comp) && (c.getShooterNum() == comp.getShooterNum())) {
                mCompetitors.remove(c);
                mCompetitors.add(comp);
                break;
            }
        }
    }

    public void updateStageInMatch(Stage stage){
        for (Stage s : mStages){
            if(stage.equals(s)){
                mStages.remove(s);
                mStages.add(stage);
                break;
            }
        }
    }


    @Override
    public boolean equals(Object o) {
        if (this == o) return true;
        if (o == null || getClass() != o.getClass()) return false;
        Match match = (Match) o;
        return Objects.equals(mMatchId, match.mMatchId);
    }

    @Override
    public int hashCode() {
        return 7 * mMatchId.hashCode();
    }

    public static String getFileName(String matchID) {
        return "match." + matchID + ".json";
    }

    public static String formatMatchDate(Date date){

        return (String) DateFormat.format("MM/dd/yyyy", date);
    }


    @Override
    public int describeContents() {
        return 0;
    }

    @Override
    public void writeToParcel(Parcel dest, int flags) {
        dest.writeSerializable(this.mMatchId);
        dest.writeString(this.mClubId);
        dest.writeString(this.mMatchName);
        dest.writeLong(this.mMatchDate != null ? this.mMatchDate.getTime() : -1);
        dest.writeInt(this.mMatchLevel == null ? -1 : this.mMatchLevel.ordinal());
        dest.writeTypedList(this.mStages);
        dest.writeTypedList(this.mCompetitors);
        dest.writeString(this.mPassword);
        dest.writeInt(this.mMatchType == null ? -1 : this.mMatchType.ordinal());
    }

    protected Match(Parcel in) {
        this.mMatchId = (UUID) in.readSerializable();
        this.mClubId = in.readString();
        this.mMatchName = in.readString();
        long tmpMMatchDate = in.readLong();
        this.mMatchDate = tmpMMatchDate == -1 ? null : new Date(tmpMMatchDate);
        int tmpMMatchLevel = in.readInt();
        this.mMatchLevel = tmpMMatchLevel == -1 ? null : MatchLevel.values()[tmpMMatchLevel];
        this.mStages = in.createTypedArrayList(Stage.CREATOR);
        this.mCompetitors = in.createTypedArrayList(Competitor.CREATOR);
        this.mPassword = in.readString();
        int tmpMMatchType = in.readInt();
        this.mMatchType = tmpMMatchType == -1 ? null : MatchType.values()[tmpMMatchType];
    }

    public static final Creator<Match> CREATOR = new Creator<Match>() {
        @Override
        public Match createFromParcel(Parcel source) {
            return new Match(source);
        }

        @Override
        public Match[] newArray(int size) {
            return new Match[size];
        }
    };
}

Here is the part of the Stage form file where I collect the fields and create the IntentService:

{
                    mStage.setTime(Double.valueOf(mTime.getText().toString()));
                    mStage.setScoringType(Stage.getScoringTypeFromString(mScoringTypeSpinner.getSelectedItem().toString()));
                    mStage.setSteelTargets(Integer.valueOf(mSteelTargets.getText().toString()));
                    mStage.setSteelNPMs(Integer.valueOf(mSteelNPMs.getText().toString()));
                    mStage.setNoShoots(mNoShoots.isChecked());

                    mStage.setRounds(mStage.getNumberOfSteelTargets());
                    mStage.setPoints(mStage.getNumberOfSteelTargets() * 5);


                    // if creating a new stage include a stage number in the stage name
                    if (!mEditing) {
                        int stageNum = (mMatch.getStages().size() + 1);
                        mStage.setStageNum(stageNum);
                        mStage.setStageName("Stage " + stageNum + ": " + mStageName.getText().toString());
                        mMatch.addStage(mStage);

                        Intent serviceIntent = WriteMatchService.getIntent(mContext, false);
                        mContext.startService(serviceIntent);

                    } else if (mEditing) {
                        // if editing an existing stage, only edit the part of the name after the stage number
                        mStage.setStageName(getNamePrefix(mStage.getStageName()) + mStageName.getText().toString());
                        mMatch.updateStageInMatch(mStage);

                        Intent serviceIntent = WriteMatchService.getIntent(mContext, false);
                        mContext.startService(serviceIntent);
                    }
                }

Once the Service is started I send the user to a different Fragment. Note: the service should run pretty quickly. I have been getting a java.util.ConcurrentModificationException, but only once in a while. This doesn't happen every time. I can't pin down why I'm getting this error. Android Studio points me to the line that says, "jsonString = new Gson().toJson(match);" in the WriteMatchToFile(Context, Match) method. What am I missing?

Here is the part of the stack trace that shows the error:

    --------- beginning of crash
2018-11-25 11:20:30.194 13665-13777/com.patgekoski.ez_score E/AndroidRuntime: FATAL EXCEPTION: IntentService[WriteMatchService]
    Process: com.patgekoski.ez_score, PID: 13665
    java.util.ConcurrentModificationException
        at java.util.ArrayList$Itr.next(ArrayList.java:860)
        at com.google.gson.internal.bind.CollectionTypeAdapterFactory$Adapter.write(CollectionTypeAdapterFactory.java:96)
        at com.google.gson.internal.bind.CollectionTypeAdapterFactory$Adapter.write(CollectionTypeAdapterFactory.java:61)
        at com.google.gson.internal.bind.TypeAdapterRuntimeTypeWrapper.write(TypeAdapterRuntimeTypeWrapper.java:69)
        at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory$1.write(ReflectiveTypeAdapterFactory.java:127)
        at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory$Adapter.write(ReflectiveTypeAdapterFactory.java:245)
        at com.google.gson.Gson.toJson(Gson.java:704)
        at com.google.gson.Gson.toJson(Gson.java:683)
        at com.google.gson.Gson.toJson(Gson.java:638)
        at com.google.gson.Gson.toJson(Gson.java:618)
        at com.patgekoski.ez_score.util.FileUtils.writeMatchToFile(FileUtils.java:75)
        at com.patgekoski.ez_score.services.WriteMatchService.onHandleIntent(WriteMatchService.java:63)
        at android.app.IntentService$ServiceHandler.handleMessage(IntentService.java:76)
        at android.os.Handler.dispatchMessage(Handler.java:106)
        at android.os.Looper.loop(Looper.java:164)
        at android.os.HandlerThread.run(HandlerThread.java:65)
2018-11-25 11:20:30.198 1699-13283/system_process W/ActivityManager:   Force finishing activity com.patgekoski.ez_score/.StagesListActivity

Answer 1

As you mentioned, the Match object contains a List<Stage>

Match objects contain a list of Stage objects.

The problem is that while Gson#toJson() is processing your Match object and converting it to a JSON, it also has to iterate over the List<Stage> using an iterator and process them as well. Sometimes (when your application fails), the iterator is being used by Gson for the conversion process and another thread modifies the List<Stage> as it happens to be a nested field of the Singleton. The iterator's next() method that is called in Gson's method throws the ConcurrentModificationException if the list has been modified since the iterator was obtained from the list.

SOLUTION
How you want to handle this failure depends on the nature of your application (how the files are saved).

One of the solutions is to use a CopyOnWriteArrayList for storing Stage objects in your Match object. It uses a copy of the List for the Iterator. No modifications to the actual list affect the Iterator.

Another can be to synchronise your methods that write the Match object to files and modify them.

Refer this post for more on ConcurrentModificationExceptions and how to avoid them.

Answer 2

I want to share what I figured out about this problem for future visitors to this post. Thanks to @Pranjal and @Ridcully for pointing me in the right direction. I wish I could give credit to you both.

Like I said, I had a global Match object (singleton) which contained a List<Stage> as a field of the Match object. While on the Stage form, I would add the newly created stage after the user presses a "Save" button, to the List<Stage> objects of the global Match. Then, I would send that Match to an IntentService (background thread) that parsed the Match object into a JSON object using the Gson library and wrote the String to a file.

While this writing was taking place in the background, the user is sent to a Fragment that consists of a RecyclerView that holds a list of the Stage objects for the current "global" Match. So the file was still writing the Match object sometimes while I was trying to extract its List<String> objects for the list on the next screen. This is where the ConcurrentModificationError came in.

2 solutions I used

1. I decided to parse the global Match into a JSON object and then send the String to the background service to write the String representation of the Match to a File while the user interacts with the next screen (the list of Stages in the singleton). This worked but it added complexity to the Fragment's code and I wanted to find a way to pass the Match object itself to the background service. So, I came up with option 2 below.

2. The solution I settled on was this: I cloned the global Match object in the background service. Now when the user is passed to the next screen, they are interacting with a different Object than the one that is being written to File, but both Objects have the same data. Problem solved.

To those who don't understand cloning, it's simple. I made my Match.class implement the Cloneable interface. Then override the method clone() like this:

 @Override
    public Object clone() throws CloneNotSupportedException {
        return super.clone();
    }

In the background service I clone the global Match like this:

if (intent.hasExtra(EXTRA_MATCH)) {
                // clone the match object to prevent a concurrentModificationException
                Match originalMatch = intent.getParcelableExtra(EXTRA_MATCH);
                try {
                    match = (Match) originalMatch.clone();
                    FileUtils.writeMatchToFile(context, match);
                } catch (CloneNotSupportedException e) {
                    e.printStackTrace();
                }

                scoresFile = intent.getParcelableExtra(EXTRA_SCORES_FILE);
                if (scoresFile != null) {
                    FileUtils.writeScoresFile(context, scoresFile);
                }
            } else {
                return;
            }

Hope this helps someone else and thank you to everyone who helped me.