0

I Have a multithreaded environment in android app. I use a singleton class to store data. This singleton class contains a arraylist that is accessed using a synchronized method.

The app uses this arraylist to render images in app.

Initial problem : Concurrent modification error use to come so I made the get arraylist function syncronized.

Current Problem:Concurrent modification error not coming but in between empty arraylist returned (maybe when there is concurrent access).

Objective : I want to detect when Concurrent modification so that Instead of empty arraylist being return I can return last state of the arraylist.

public synchronized List<FrameData> getCurrentDataToShow() {
    List<FrameData> lisCurrDataToShow = new ArrayList<FrameData>();
    //for (FrameData fd : listFrameData) {//concurrent modification exception
    //todo iterator test
    Iterator<FrameData> iterator = listFrameData.iterator();
    while (iterator.hasNext()) {
        FrameData fd = iterator.next();
        long currentTimeInMillis = java.lang.System.currentTimeMillis();

        if ((currentTimeInMillis > fd.getStartDate().getTime() && currentTimeInMillis < fd.getEndDate().getTime()) || (fd.isAllDay() && DateUtils.isToday(fd.getStartDate().getTime()))) {
            if (new File(ImageFrameActivity.ROOT_FOLDER_FILES + fd.getFileName()).exists()) {
                lisCurrDataToShow.add(fd);
            }
        }
    }
    if (lisCurrDataToShow.size() == 0) { 
        lisCurrDataToShow.add(new FrameData(defaultFileName, null, null, null, String.valueOf(120), false));
    }
    return lisCurrDataToShow;
}

Referred to Detecting concurrent modifications?

Please help!

EDIT1:

This problem occurs rarely not everytime.

  • If a threads is accessing getCurrentDataToShow() and another thread tries to access this function what will the function return?? I'm new to multithreading , please guide

Edit 2

in oncreate following methods of singleton are called periodically

DataModelManager.getInstance().getCurrentDataToShow(); DataModelManager.getInstance().parseData(responseString);

Complete singleton class

public class DataModelManager {
    private static DataModelManager dataModelManager;
    private ImageFrameActivity imageFrameAct;
    private String defaultFileName;
    public List<FrameData> listFrameData = new ArrayList<FrameData>();
//    public CopyOnWriteArrayList<FrameData> listFrameData= new CopyOnWriteArrayList<FrameData>();
    private String screensaverName;
    private boolean isToDownloadDeafultFiles;
    private String tickerMsg = null;
    private boolean showTicker = false;
    private boolean showHotspot = false;
    private String hotspotFileName=null;
    public String getDefaultFileName() {
        return defaultFileName;
    }
    public boolean isToDownloadDeafultFiles() {
        return isToDownloadDeafultFiles;
    }
    public void setToDownloadDeafultFiles(boolean isToDownloadDeafultFiles) {
        this.isToDownloadDeafultFiles = isToDownloadDeafultFiles;
    }
    private String fileNames;
    private DataModelManager() {
    }
    public static DataModelManager getInstance() {
        if (dataModelManager == null) {
            synchronized (DataModelManager.class) {
                if (dataModelManager == null) {
                    dataModelManager = new DataModelManager();
                }
            }
        }
        return dataModelManager;
    }
    private synchronized void addImageData(FrameData frameData) {
        //Log.d("Frame Data","Start date "+frameData.getStartDate()+ " " +"end date "+frameData.getEndDate());
        listFrameData.add(frameData);
    }
    public synchronized void parseData(String jsonStr) throws JSONException {
        listFrameData.clear();
        if (jsonStr == null) {
            return;
        }
        List<String> listFileNames = new ArrayList<String>();
        JSONArray jsonArr = new JSONArray(jsonStr);
        int length = jsonArr.length();
        for (int i = 0; i < length; i++) {
            JSONObject jsonObj = jsonArr.getJSONObject(i);
            dataModelManager.addImageData(new FrameData(jsonObj.optString("filename", ""), jsonObj.optString("start", ""), jsonObj.optString("end", ""), jsonObj.optString("filetype", ""), jsonObj.optString("playTime", ""), jsonObj.optBoolean("allDay", false)));
            listFileNames.add(jsonObj.optString("filename", ""));
        }
        fileNames = listFileNames.toString();
    }
    public void setDefaultFileData(String jsonStr) throws JSONException {
        JSONObject jsonObj = new JSONObject(jsonStr);
        defaultFileName = jsonObj.optString("default_image", "");
        screensaverName = jsonObj.optString("default_screensaver ", "");
    }
    @Override
    public String toString() {
        return fileNames.replace("[", "").replace("]", "") + "," + defaultFileName + "," + screensaverName;
    }
    public FrameData getFrameData(int index) {
        return listFrameData.get(index);
    }
    public synchronized List<FrameData> getCurrentDataToShow() {
        List<FrameData> lisCurrDataToShow = new ArrayList<FrameData>();
//        for (FrameData fd : listFrameData) {//concurrent modification exception
            //todo iterator test
            Iterator<FrameData> iterator = listFrameData.iterator();
            while (iterator.hasNext()) {
                FrameData fd = iterator.next();
            long currentTimeInMillis = java.lang.System.currentTimeMillis();
            if ((currentTimeInMillis > fd.getStartDate().getTime() && currentTimeInMillis < fd.getEndDate().getTime()) || (fd.isAllDay() && DateUtils.isToday(fd.getStartDate().getTime()))) {
                if (new File(ImageFrameActivity.ROOT_FOLDER_FILES + fd.getFileName()).exists()) {
                    lisCurrDataToShow.add(fd);
                }
            }
        }
        if (lisCurrDataToShow.size() == 0) {
            lisCurrDataToShow.add(new FrameData(defaultFileName, null, null, null, String.valueOf(120), false));
        }
        return lisCurrDataToShow;
    }
    public String getCurrentFileNames() {
        String currFileNames = "";
        List<FrameData> currFrameData = getCurrentDataToShow();
        for (FrameData data : currFrameData) {
            currFileNames += "," + data.getFileName();
        }
        return currFileNames;
    }
    public ImageFrameActivity getImageFrameAct() {
        return imageFrameAct;
    }
    public void setImageFrameAct(ImageFrameActivity imageFrameAct) {
        this.imageFrameAct = imageFrameAct;
    }

}
Community
  • 1
  • 1
Rachita Nanda
  • 4,509
  • 9
  • 42
  • 66
  • Does this method have *exclusive* access to `listFrameData`? You need to make sure that no other method can modify it at the same time, i.e. all methods which modify it need to be synchronized on the same monitor. – Andy Turner Nov 30 '15 at 08:25
  • You might want to look at something like [`CopyOnWriteArrayList`](https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/CopyOnWriteArrayList.html) to handle concurrent modifications, e.g. if "traversal operations vastly outnumber mutations". – Andy Turner Nov 30 '15 at 08:27
  • I tried using CopyOnWriteArrayList ,it was causing this situation of empty arraylist returned more often – Rachita Nanda Nov 30 '15 at 09:27
  • I would expect you to get a `ConcurrentModificationException` whether you use an explicit iterator or enhanced for statement, since the latter is syntactic sugar for the former. Are you sure that your list isn't simply empty? Are you assigning a new value to `listFrameData` somewhere? – Andy Turner Nov 30 '15 at 09:28
  • Yes. I'm sure list is not empty . When the this function was not synchronized concurrent modification was thrown now intstead this if (lisCurrDataToShow.size() == 0) { //.. } is hapening .Maybe the arraylist is not being accessed as some other thread is accessing it. – Rachita Nanda Nov 30 '15 at 09:31
  • I'm afraid that your descriptions are too vague, and it is unclear how the code you have supplied relates to them. For instance, the code you have supplied cannot possibly return an empty list. Please provide an MVCE - http://stackoverflow.com/help/mcve – Stephen C Nov 30 '15 at 11:34

1 Answers1

0

This is the only part of your question that is currently answerable:

If a threads is accessing getCurrentDataToShow() and another thread tries to access this function what will the function return?

It depends on whether you are calling getCurrentDataToShow() on the same target object; i.e. what this is.

  • If this is the same for both calls, then the first call will complete before the second call starts.

  • If this is different, you will be locking on different objects, and the two calls could overlap. Two threads need to lock the same object to achieve mutual exclusion.

  • In either case, this method is not changing the listFrameData collection. Hence it doesn't matter whether the calls overlap! However, apparently something else is changing the contents of the collection. If that code is not synchronizing at all, or if it is synchronizing on a different lock, then that could be a source of problems.

Now you say that you are not seeing ConcurrentModificationException's at the moment. That suggests (but does not prove) that there isn't a synchronization problem at all. And that suggests (but does not prove) that your current problem is a logic error.

But (as I commented above) there are reasons to doubt that the code you have shown us is an true reflection of your real code. You need to supply an MVCE if you want a more definite diagnosis.

Stephen C
  • 698,415
  • 94
  • 811
  • 1,216
  • Thanks for your response. I have edited my question and added the complete singleton class. I think I'm not able to manage the multi threaded environment with singleton. I'm new in multi threading please guide – Rachita Nanda Nov 30 '15 at 12:18
  • I suspect that the problem is that `parseData` is sometimes called with an null string or a string consisting of an empty JSON array. But this is still not an MVCE. We can't run this code to demonstrate the problem. – Stephen C Nov 30 '15 at 12:39