Java – Synchronization of lockCanvas and unlockCanvasAndPost for SurfaceHolder

I

know there are a lot of questions on this topic, however, I am still not entirely satisfied with the answers provided.

Situation:
I implemented SurfaceView by drawing in another thread using SurfaceHolder, as suggested in the developer guide: http://developer.android.com/guide/topics/graphics/2d-graphics.html

Question:
Sometimes I get java.lang.IllegalStateException: Surface has already been released:

• When calling SurfaceHolder.lockCanvas().
• Or when calling SufraceHolder.unlockCanvasAndPost().

This means that my surface is sometimes released before locking the Canvas, and sometimes between locking and unlocking.

My solution:
I perform surface checks and lock/unlock Canvas in a synchronized block, so I’m sure the surface won’t be broken between these operations. However, I’ve never used a synchronized block, and I’d like to ask if there’s anything wrong with it. The code works fine so far, but you never know when sync issues will arise, so I’m not entirely convinced that this is the best way to go.

private class DrawingThread extends Thread{
    @Override
    public void run() {
        super.run();
        while (!isInterrupted() && holder != null) {
            Canvas drawingCanvas = null;
            synchronized (this) {
                if (holder.getSurface().isValid()) {
                    drawingCanvas = holder.lockCanvas();
                }
            }
            if (drawingCanvas != null && drawingCanvas.getWidth() > 0) {
                drawThisView(drawingCanvas);
                synchronized (this) {
                    if(holder.getSurface().isValid()) {
                        holder.unlockCanvasAndPost(drawingCanvas);
                    }
                }
            }
        }
    }
}

@Override
public void surfaceCreated(SurfaceHolder holder) {
    if(drawingThread != null){
        drawingThread.interrupt();
    }
    drawingThread = new DrawingThread();
    drawingThread.start();
}

@Override
public void surfaceChanged(SurfaceHolder holder, int format, int width, int height) {
    if(drawingThread.isInterrupted()){
        drawingThread = new DrawingThread();
        drawingThread.start();
    }
}

@Override
public void surfaceDestroyed(SurfaceHolder holder) {
    drawingThread.interrupt();
}

The synchronized statement is used for exclusive access between multiple threads. If you synchronize (this) in thread #1, then any other thread that tries to do the same will block until thread #1 exits the synchronized block.

If you only use it in one thread, it is of no use. Of course, if you have multiple threads trying to lock and unlock your Surface’s Canvas, you should not do so in the first place to fix the problem, rather than trying to enforce exclusive access after the fact.

Your use of interrupt() and isThreadInterrupted() doesn’t make much sense. If you want to set a flag to tell the thread that it’s time to stop running, volatile boolean will do. The only advantage of interrupt() is that it wakes up while the thread is waiting for the object to be signaled. Also, consider the documentation for the isInterrupted() call:

Returns a boolean indicating whether the receiver has a pending interrupt request (true) or not ( false)

(Emphasis mine.) Your code allows for the situation where, “If a thread is in the Active state but an interrupt has been thrown for it, go ahead and create a new thread while the old thread is still running.”

Some information about using SurfaceView can be found in the this appendix . This links to an example in Grafika that starts and stops threads based on Surface callbacks, which you can find here .