Synchronization of SurfaceHolder's lockCanvas and unlockCanvasAndPost

Question

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

The situation: I implemented SurfaceView with drawing in another thread using SurfaceHolder like it is suggested in the developer's guide: http://developer.android.com/guide/topics/graphics/2d-graphics.html

The problem: 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 I lock the canvas, and sometimes - between locking and unlocking it.

My solution: I perform surface check and locking/unlocking canvas within synchronized blocks, so I know for sure that the surface can't get destroyed in between those actions. However, I have never worked with synchronized blocks and I wanted to ask if there is anything wrong with it. So far the code worked fine, but you never know when synchronization issues may show themselves, so I'm not completely convinced that this is the best way.

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();
}

Answer 1

After some hours of trials and errors I think I have figured it out. After all, all I had to do is to read the android reference. Here are some important things that I got wrong:

• SurfaceHolder.getSurface.isValid() only checks if the surface can be locked, so this check can (and does) fail for seeing is the canvas can be unlocked.
• To check if you can call unlockCanvasAndPost(), you just check if the canvas returned by lockCanvas() is not null.
• If the canvas is null, you should not unlock it (if you try it will throw an exception).
• If it's not null, you have to unlock it, otherwise your app will freeze if you activity tries to stop (after onPause() the SurfaceHolder tries to destroy the Surface, but since the Canvas is locked, it couldn't, so you get into a deadlock).

Hope this can help other "noobs". Finally, this is my final code for SurfaceView(the try/catch block is not necessary any more):

DrawingThread drawingThread;

private class DrawingThread extends Thread{
    public volatile boolean canDraw = true;

    @Override
    public void run() {
        try {
            while (canDraw) {
                Canvas drawingCanvas = null;
                if (canDraw && holder.getSurface().isValid()) {
                    drawingCanvas = holder.lockCanvas();
                    if (drawingCanvas != null) {
                        drawThisView(drawingCanvas);
                        holder.unlockCanvasAndPost(drawingCanvas);
                    }
                }
            }
        }catch(IllegalStateException e){
            e.printStackTrace();
            canDraw = false;
        }
    }
}


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

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

@Override
public void surfaceDestroyed(SurfaceHolder holder) {
    if(drawingThread != null) {
        drawingThread.canDraw = false;
    }
}

And thanks fadden for clarifying some points about drawing in another thread.