0

I have an AsyncTask that queries a content provider and does some additional processing in the onPostExecute() of that task. I have a stack trace for an exception that is very hard to reproduce, but I would like to guard against the condition or fix the code. The code is below:

int i = 0;
mIds = new long[cursor.getCount()];
while (cursor.moveToNext()) {
    mIds[i++] = cursor.getLong(COLUMN_ID);
}

The crash is happening on the line in the loop. The way I see it, the only way this could happen is if:

  1. cursor.getCount() is returning an incorrect count.
  2. cursor is changed while this loop is executing, but I don't think that's possible because cursor is a local variable. Perhaps something underlying the cursor has changed that I'm not aware of.
  3. mIds has changed. This shouldn't be possible because we are running on the UI thread and this is the only place in which that variable is assigned a new value. By the nature of onPostExecute running on the UI thread, it shouldn't be possible for this code to be running somewhere else at the same time, right?

Am I missing something?

Steve Prentice
  • 23,230
  • 11
  • 54
  • 55
  • Log the length of `mIds` just before the loop. Then place a conditional break at the assignment to mIds[i++] and break if `i >= mIds.length`. When it breaks, you can check whether `mIds` was reassigned somehow, whether `cursor` changed length, or what else might be going on. – Ted Hopp Nov 14 '11 at 20:24
  • I don't currently log this information and have been unable to reproduce (got one bug report), so I'm hoping I can add code to guard against the condition. – Steve Prentice Nov 14 '11 at 20:29

3 Answers3

1

try this:

int i = 0;
mIds = new long[cursor.getCount()];
while (cursor.moveToNext()) {
    mIds[i] = cursor.getLong(COLUMN_ID);
    i++;
}
owen gerig
  • 6,165
  • 6
  • 52
  • 91
  • Can you elaborate on why you think moving the i++ will make a difference here? – Steve Prentice Nov 14 '11 at 20:28
  • well i believe its adding before being used. so hence it would start at 1 (as opposed to 0) and end at +1 over where it should be. i could be wrong and dont have a compiler in front of me to test – owen gerig Nov 14 '11 at 20:31
1

It's difficult to see what is wrong with that loop without seeing the full code but I note from the docs:

Cursor implementations are not required to be synchronized so code using a Cursor from multiple threads should perform its own synchronization when using the Cursor.

D-Dᴙum
  • 7,689
  • 8
  • 58
  • 97
  • Out of all the answers, I think this one is the closest. It made me go look a little deeper at the code. The loop is actually in a method called from `MyAdapter.changeCursor(cursor)` AND the method that contains the loop is called AFTER `super.changeCursor(cursor)`. `changeCursor()` is the method that is called in `onPostExecute()`. Now, my thinking is that after the `super.changeCursor()` the cursor becomes available to other threads accessing the adapter and those threads must be resetting the position during my loop. – Steve Prentice Nov 14 '11 at 20:56
0

Drawing from my experiences, I always do it like this. Seems to avoid most problems.

Cursor cursor = null;

try
{
    cursor = getDb().rawQuery(query, params); // your call to database here...
    if (cursor == null || cursor.getCount() == 0)
        return false;

    cursor.moveToFirst();

    mIds = new long[cursor.getCount()];
    int i = 0;

    do
    {
        mIds[i] = cursor.getLong(COLUMN_ID);
        i++;
    } while (cursor.moveToNext())
} catch(Exception e) {
    // Do error handling
    return false;
}
finally
{
    closeCursor(cursor);
    cursor = null;
}

return true;

Here's backup for my approach:
Is Android Cursor.moveToNext() Documentation Correct?
Does finally always execute in Java?

Community
  • 1
  • 1
Jarno Argillander
  • 5,885
  • 2
  • 31
  • 33
  • Thanks. That'll avoid the force close, but I'm more interested in understanding why this exception is happening than masking it. – Steve Prentice Nov 14 '11 at 20:47
  • Yeah, it's always good to understand. Anyway, I recall I had issues with the `moveToNext()`... Try this: add `moveToFirst()` before while, then change while to do-while. See if that helps. Plus I'd also take the `i++` away from the `mIds[i++]`. – Jarno Argillander Nov 14 '11 at 21:05