1

In my tests I see the following exception in updateOomAdjLocked() of ActivityManagerService:

// java.lang.IndexOutOfBoundsException: Invalid index 27, size is 27
//  at java.util.ArrayList.throwIndexOutOfBoundsException(ArrayList.java:251)
//  at java.util.ArrayList.get(ArrayList.java:304)
//  at com.android.server.am.ActivityManagerService.updateOomAdjLocked(ActivityManagerService.java:13880)
//  at com.android.server.am.ActivityManagerService.updateLruProcessLocked(ActivityManagerService.java:1904)
//  at com.android.server.am.ActivityStack.realStartActivityLocked(ActivityStack.java:647)
//  at com.android.server.am.ActivityStack.startSpecificActivityLocked(ActivityStack.java:803)

Problematic code seems is the following (line 13850 for Android 4.2.2 r1.2):

final ArrayList<ProcessRecord> mLruProcesses
    = new ArrayList<ProcessRecord>();
{...}

final void updateOomAdjLocked() {
    {...}
    final int N = mLruProcesses.size();
    for (i=0; i<N; i++) {
        ProcessRecord app = mLruProcesses.get(i);
        {...}
    }

If mLruProcesses.remove was called while in the loop, then mLruProcesses.get(i) for i=N accessed a non-existing index, triggering an exception.

I know very little about Android services so far, so my question is if this code needs to be reenterable, perhaps by making some of the operations thread-safe?

Andrey
  • 13
  • 2

1 Answers1

2

You could verify that the size of mLruProcess.size() is still equal to N within your loop.

final boolean updateOomAdjLocked() {
   {...}
   final int N = mLruProcess.size();
   for(int i = 0; i < N; i++) {
       if(N != mLruProcess.size()) {
           //Do something accordingly, in this case return false
           return false;
       }

       ProcessRecord app = mLruProcess.get(i);
       {...}
   }

   {...}       

   return true;
}

Add some logic where you call this function, if it returns false try to have it call the function again. Otherwise, you could put mLruProcess in a Synchronized block which means that it can be accessed in only one concurrent thread. The Synchronized keyword is blocking, which means any code that tries to access mLruProcess in another thread will be blocked until the current thread is done with mLruProcess.

If you don't like to, or can't use blocking code, try using an AtomicBoolean as shown here:

When to use synchronized in Java

Set the AtomicBoolean whenever you change the object stored in mLruProcess and then check that AtomicBoolean everywhere you need to access the objects stored in mLruProcess.

Community
  • 1
  • 1
David Freitag
  • 2,252
  • 2
  • 16
  • 18
  • OP's snippet comes from the [Android source code](https://github.com/android/platform_frameworks_base/blob/master/services/java/com/android/server/am/ActivityManagerService.java). I don't think he can modify those internals, rather he's looking for solutions to avoid this (seemingly unintended) concurrency issue. – Mattias Buelens Jul 09 '13 at 18:44
  • Thanks, David and Mattias. Yes, there are a few ways to avoid this exception and each one has to be evaluated for not breaking the OOM adj logic. Mostly I was wondering if this is supposed to be re-enterable code that is aware of the concurrency issues. In the way it is done by Google, doesn't seem like it is. The fact that I'm hitting this exception, seems it has to be. – Andrey Jul 09 '13 at 18:59
  • Because this method is declared `final`, it can't be modified. At the very least the OP could look into `AtomicBoolean`s to try and solve his/her issues. – David Freitag Jul 09 '13 at 18:59