Search code examples
androidmultithreadinglinked-listsurfaceviewconcurrentmodification

Android LinkedList ConcurrentModificationException SurfaceView Thread


I have a SurfaceView that that the user can draw multiple bitmaps to and modify (stickers). The stickers are held in a LinkedList that is iterated on MotionEvent.ACTION_DOWN to find which sticker is being touched by the user:

private void setActiveSticker(float x, float y) {
    Iterator<Sticker> stickersDesc = mStickers.descendingIterator();
    while (stickersDesc.hasNext()) {
        Sticker sticker = stickersDesc.next();
        if (sticker.collider(x, y)) {
            mActiveSticker = sticker;
            mMode = MODE_DRAG;
            break;
        }
        mStickers.remove(mActiveSticker);
        mStickers.add(mActiveSticker);
    }
}

This LinkedList is also iterated to draw each one to the Canvas of the SurfaceView as they are manipulated:

@Override
public void draw (Canvas canvas) {
    super.draw(canvas);

    canvas.drawBitmap(mBitmap, mMatrix, mPaint);

    for (Sticker sticker : mStickers) {
        sticker.draw(canvas);
    }
}

And this is where I get a ConcurrentModificationException:

09-28 08:56:41.769  19832-24370/com.example.ex E/AndroidRuntime﹕ FATAL EXCEPTION: Thread-5279
Process: com.example.ex, PID: 19832
java.util.ConcurrentModificationException
        at java.util.LinkedList$LinkIterator.next(LinkedList.java:124)
        at com.example.ex.utilities.DrawingSurface.draw(DrawingSurface.java:133)
        at com.example.ex.utilities.DrawingThread.onSurfaceUpdate(DrawingThread.java:95)
        at com.example.ex.utilities.DrawingThread.run(DrawingThread.java:46)

The draw() method of the SurfaceView is called by a separate Thread:

public class DrawingThread extends Thread {
    volatile boolean mRunning = false;

    private long mRefreshRate;
    private DrawingSurface mSurface;

    public DrawingThread (DrawingSurface surface, long time) {
        super();
        mSurface = surface;
        mRefreshRate = time;
    }

    public void setRunning (boolean run) {
        mRunning = run;
    }

    @Override
    public void run() {
        while (mRunning) {
            try {
                sleep(mRefreshRate);
                onSurfaceUpdate();
            } catch (InterruptedException exception) {
                exception.printStackTrace();
            }
        }
    }

    public void onSurfaceChanged(Configuration config, Point fit, float ratio) {
        float width, height;
        if (config.orientation == Configuration.ORIENTATION_LANDSCAPE) {
            width = fit.y * ratio;
            height = fit.y;
        } else if (config.orientation == Configuration.ORIENTATION_PORTRAIT) {
            width = fit.x;
            height = fit.x / ratio;
        } else {
            width = fit.x;
            height = fit.x / ratio;
        } mSurface.getHolder().setFixedSize((int) width, (int) height);
    }

    private void onSurfaceUpdate() {
        Canvas canvas = null;

        try {
            canvas = mSurface.getHolder().lockCanvas();
            synchronized (mSurface.getHolder()) {
                mSurface.draw(canvas);
            }
        } finally {
            if (canvas != null) {
                mSurface.getHolder().unlockCanvasAndPost(canvas);
            }
        }
    }
}

I have tried pausing the thread before the LinkedList is iterated in setActiveSticker() and resuming after the loop is finished in an attempt to avoid the both modifications from happening at the same time. Even though that does not seem to be recommended. I would like to know how I can iterate the LinkedList without this error, or if there is a better way of achieving the same functionality.


Solution

  • I have found a solution. Instead of iterating the LinkedList, an Iterator, or a ListIterator in draw(), which can all produce a ConcurrentModificationException, I converted the LinkedList to a simple array, like so:

    Sticker[] stickers = mStickers.toArray(new Sticker[mStickers.size()]);
    for(Sticker sticker : stickers) {
        canvas.drawBitmap(sticker.getBitmap(), sticker.getMatrix(), sticker.getPaint());
    }
    

    I also left setActiveSticker() method as I originally posted it, considering it was not producing any errors. I found the answer I was looking for in a small list of options titled "To Avoid ConcurrentModificationException In [A] Multi-Threaded Environment" in this article: How To Avoid ConcurrentModificationException When Using An Iterator.

    EDIT: My new draw method based on the tip from @fadden:

    public void drawSurface(Canvas canvas) {
        canvas.drawBitmap(mBitmap, mMatrix, mPaint);
    
        Sticker[] stickers = mStickers.toArray(new Sticker[mStickers.size()]);
        for (Sticker sticker : stickers) {
            canvas.drawBitmap(sticker.getBitmap(), sticker.getMatrix(), sticker.getPaint());
        }
    }