Concurrent Modification Exception, despite waiting for finish

Question

Here's a section of my onCreate, which sometimes is causing exception:

public void onCreate(Bundle savedInstanceState) {
    super.onCreate(savedInstanceState);
    setContentView(R.layout.activity_tilisting);
    _context = getApplicationContext();
    SDName = Environment.getExternalStorageDirectory();
    //listview = (ListView)findViewById(R.id.TIlistview);
    String TIdir = new File(SDName, "/TitaniumBackup/").toString();
    final ArrayList<String> apps = new ArrayList<String>();
    final StringBuffer done = new StringBuffer();
    Command command = new Command(0,"ls -a "+TIdir+"/*.properties") {
        @Override
        public void output(int arg0, String arg1) {
            synchronized(apps) {
                apps.add(arg1);
                if (!done.toString().equals("")) {
                    done.append("done");//oh no
                }
            }
        }
    };
    try {
        RootTools.getShell(true).add(command).waitForFinish();
        String attrLine = "";
        int ind;
        backups = new ArrayList<TIBackup>();
        synchronized(apps) {
            for (String app : apps) {
                try {
                    TIBackup bkup = new TIBackup(app);
                    FileInputStream fstream = new FileInputStream(app);
                    BufferedReader atts = new BufferedReader(new InputStreamReader(fstream));
                    while ((attrLine = atts.readLine()) != null) {
                        ind = attrLine.indexOf('=');
                        if (ind !=-1 && !attrLine.substring(0,1).equals("#"))
                        bkup.prop.put(attrLine.substring(0,ind), attrLine.substring(ind+1));
                    }
                    backups.add(bkup);
                    atts.close();
                } catch (FileNotFoundException e) {
                    // TODO Auto-generated catch block
                    e.printStackTrace();
                } catch (IOException e) {
                    // TODO Auto-generated catch block
                    e.printStackTrace();
                }
            }
            done.append("done");
        }
        setListAdapter( new StableArrayAdapter(this,backups));
    } catch (InterruptedException e) {
        //TODO:errors
        e.printStackTrace();
    } catch (IOException e) {
        e.printStackTrace();
    } catch (TimeoutException e) {
        e.printStackTrace();
    }

The for (String app : apps) { is causing the exception, despite the waitforfinish() before it.

This updated code should fix it, adding data from the output, and waiting for any stragglers with the synchronized in the main code, but if you set a breakpoint on the //oh no line above, it is still getting to this point where it tries to add an item after the UI main code ran. So waitforfinish() is not waiting? How do I prevent this race condition?

I also tried the RootTask code below, but it seems to stop at the last readline?

    RootTask getProfile = new RootTask() {
        @Override
        public void onPostExecute(ArrayList<String> result) {
            super.onPostExecute(result);
            for (String r : result) {
                System.out.println(r);
            }
        }
    };
    getProfile.execute("ls /data/data/org.mozilla.firefox/files/mozilla/" );

onPostExecute never runs.

Answer 1

This was partially caused by a design flaw in RootTools. I believe the crux of the issue is that the operation that you are performing on the shell is taking longer than the default timeout that is set for shell commands. When the timeout occurs it simply returns the command as completed which is where the design flaw lies.

I have provided a new jar to use as well as some more information on this. I have also deprecated waitForFinish() as I agree that it was, and is, a poor solution.

https://code.google.com/p/roottools/issues/detail?id=35

Please let me know if you have any questions or problems :)

Answer 2

Output() is to be called during waitForFinish() waits. Something is wrong in the code implementing Command execution.

Most likely: the command executor (RootTools ?) runs the command on shell, gets a bunch of output lines, notifies the calling thread from waiting, and then calls output() of command for each line it got as output. I think it should notify the command thread after output() has been called on command object, for all output lines.

Still you can wrap the list modifying code and list iterating code in synchronized(<some common object>){}.

Update:

So waitForFinish() is not waiting? How do I prevent this race condition?

It does wait, but not for your code. Synchronized keyword merely made sure that output() of Command object is not called at the same time when you are iterating the apps collection. It does not schedule the two threads to run in a particular sequence.

IMHO, waitForFinish() is not a good pattern, making calling thread waiting defeats the point of a separate executor. It better be formulated like an AsyncTask or accept an event listener for each Command object.

Just a rough example, this class:

public class RootTask extends AsyncTask<String,Void,List<String>> {
    private boolean mSuccess;

    public boolean isSuccess() {
        return mSuccess;
    }

    @Override
    protected List<String> doInBackground(String... strings) {
        List<String> lines = new ArrayList<String>();

        try {
            Process p = Runtime.getRuntime().exec("su");
            InputStream is = p.getInputStream();
            OutputStream os = p.getOutputStream();

            os.write((strings[0] + "\n").getBytes());

            BufferedReader rd = new BufferedReader(new InputStreamReader(is));

            String line;

            while ((line = rd.readLine()) != null){
                lines.add(line);
            }

            mSuccess = true;
            os.write(("exit\n").getBytes());
            p.destroy();

        } catch (IOException e) {
            mSuccess = false;
            e.printStackTrace();
        }

        return lines;
    }
}

can be used as:

RootTask listTask = new RootTask{
  @Override
  public void onPostExecute(List<String> result){
      super.onPostExecute();
      apps.addAll(result);
      //-- or process the results strings--
  }
};

listTask.execute("ls -a "+TIdir+"/*.properties");