Obsolete
Status Update
No update yet.
Comments
an...@gmail.com
I've tried this on a HTC Sensation XE 4.0.3 with Lunar Lander, out the box, no modification, and I also see the same behaviour where a null canvas is incorrectly returned before the surface is destroyed. Net result, the sample game crashes when rotated.
mk...@gmail.com
I wasted a good amount of time tracking this down because I didn't expect it to be a bug in android itself. Is there a consensus on the workarounds suggested? Is it considered acceptable practice to catch NullPointerException (which could be thrown from other bugs in the draw method)? Also, is it safe to condition the call to the draw method on canvas being not null (i.e. is it safe to assume there no chance that it will become null in the middle of the draw method)?
er...@gmail.com
[Comment deleted]
er...@gmail.com
There is a bug in the run function. You need to verify if c is not null before drawing.
So you should implement the folowing code to avoid the bug:
@Override
public void run() {
while (mRun) {
Canvas c = null;
try {
c = mSurfaceHolder.lockCanvas(null);
if (c!=null){
synchronized (mSurfaceHolder) {
if (mMode == STATE_RUNNING) updatePhysics();
doDraw(c);// draw it
}
}
} finally {
// do this in a finally so that if an exception is thrown
// during the above, we don't leave the Surface in an
// inconsistent state
if (c != null) {
mSurfaceHolder.unlockCanvasAndPost(c);
}
}
}
}
So you should implement the folowing code to avoid the bug:
@Override
public void run() {
while (mRun) {
Canvas c = null;
try {
c = mSurfaceHolder.lockCanvas(null);
if (c!=null){
synchronized (mSurfaceHolder) {
if (mMode == STATE_RUNNING) updatePhysics();
doDraw(c);// draw it
}
}
} finally {
// do this in a finally so that if an exception is thrown
// during the above, we don't leave the Surface in an
// inconsistent state
if (c != null) {
mSurfaceHolder.unlockCanvasAndPost(c);
}
}
}
}
cr...@gmail.com
Eric, this solution was provided in my original post. It also doesn't change the fact that the underlying behavior changed across versions. Even the samples were assuming a non-null canvas. The better solution is to fix the samples, as then this error won't creep into everyone else's code.
xi...@gmail.com
Agree. I am having the same problem on my Nexus 4.
en...@google.com
pa...@gmail.com
I also had suffered from such issue. but i found result on .
only little change in code will help .
public CrashSurfaceView (Context context) {
super(context);
getHolder().addCallback(this);
setFocusable(true);
}
public CrashSurfaceView (Context context, AttributeSet attrs) {
super(context, attrs);
getHolder().addCallback(this);
setFocusable(true);
}
public CrashSurfaceView (Context context, AttributeSet attrs, int defStyle) {
super(context, attrs, defStyle);
getHolder().addCallback(this);
setFocusable(true);
}
public void surfaceCreated(SurfaceHolder holder) {
thread = new MapThread(getHolder(), this);
thread.setRunning(true);
thread.start();
}
@Override
public void surfaceDestroyed(SurfaceHolder holder) {
boolean retry = true;
thread.setRunning(false);
while (retry) {
try {
thread.join();
retry = false;
} catch (InterruptedException e) {
}
}
}
@Override
public void run() {
Canvas canvas;
while (run) {
canvas = null;
try {
canvas= surfaceHolder.lockCanvas(null);
synchronized (surfaceHolder) {
//call methods to draw and process next fame
CrashSurfaceView.onDraw(canvas);
}
} finally {
if (canvas!= null) {
surfaceHolder.unlockCanvasAndPost(canvas);
}
}
}
}
only little change in code will help .
public CrashSurfaceView (Context context) {
super(context);
getHolder().addCallback(this);
setFocusable(true);
}
public CrashSurfaceView (Context context, AttributeSet attrs) {
super(context, attrs);
getHolder().addCallback(this);
setFocusable(true);
}
public CrashSurfaceView (Context context, AttributeSet attrs, int defStyle) {
super(context, attrs, defStyle);
getHolder().addCallback(this);
setFocusable(true);
}
public void surfaceCreated(SurfaceHolder holder) {
thread = new MapThread(getHolder(), this);
thread.setRunning(true);
thread.start();
}
@Override
public void surfaceDestroyed(SurfaceHolder holder) {
boolean retry = true;
thread.setRunning(false);
while (retry) {
try {
thread.join();
retry = false;
} catch (InterruptedException e) {
}
}
}
@Override
public void run() {
Canvas canvas;
while (run) {
canvas = null;
try {
canvas= surfaceHolder.lockCanvas(null);
synchronized (surfaceHolder) {
//call methods to draw and process next fame
CrashSurfaceView.onDraw(canvas);
}
} finally {
if (canvas!= null) {
surfaceHolder.unlockCanvasAndPost(canvas);
}
}
}
}
Description
The documentation for Callback.surfaceDestroyed says "This is called immediately before a surface is being destroyed. After returning from this call, you should no longer try to access this surface. If you have a rendering thread that directly accesses the surface, you must ensure that thread is no longer touching the Surface before returning from this function."
This language implies that if we try to join on our rendering thread in surfaceDestroyed, our rendering thread should continue to get a touchable surface until it has a chance to terminate its loop. This is the approach taken in the Lunar Lander example. However, for me and for several others, lockCanvas returns null on Android 4.0.3 on my Samsung Galaxy S3 and my Asus Transformer. Canvas is non-null on earlier versions.
Other cases of this problem that I found include:
-
-
-
To reproduce the problem:
1) Create an Activity whose View is:
import android.content.Context;
import android.graphics.Canvas;
import android.graphics.Color;
import android.util.AttributeSet;
import android.util.Log;
import android.view.SurfaceHolder;
import android.view.SurfaceView;
public class CrashSurfaceView extends SurfaceView implements SurfaceHolder.Callback {
private UpdateThread updater;
public CrashSurfaceView(Context context,
AttributeSet attrs) {
super(context, attrs);
getHolder().addCallback(this);
}
@Override
public void surfaceChanged(SurfaceHolder holder,
int format,
int width,
int height) {
Log.d("FOO", "surface changed to " + width + ", " + height);
}
@Override
public void surfaceCreated(SurfaceHolder holder) {
Log.d("FOO", "surface created");
updater = new UpdateThread(getHolder());
updater.start();
}
@Override
public void surfaceDestroyed(SurfaceHolder holder) {
Log.d("FOO", "surface destroyed");
boolean isJoined = false;
updater.kill();
while (!isJoined) {
try {
updater.join();
isJoined = true;
} catch (InterruptedException e) {
}
}
Log.d("FOO", "thread joined");
}
class UpdateThread extends Thread {
private SurfaceHolder holder;
private boolean isRequestedToStop;
public UpdateThread(SurfaceHolder holder) {
this.holder = holder;
isRequestedToStop = false;
}
public void kill() {
isRequestedToStop = true;
}
@Override
public void run() {
while (!isRequestedToStop) {
Canvas canvas = null;
try {
canvas = holder.lockCanvas();
synchronized (holder) {
canvas.drawColor(Color.GREEN); // NPE here 'cuz canvas is null
}
} finally {
if (canvas != null) {
holder.unlockCanvasAndPost(canvas);
}
}
}
}
}
}
2) Rotate the device or pause the app to cause the surface to be destroyed.
3) Observe a NullPointerException on the line above marked "// NPE here..."
Workarounds include checking canvas for null and catching a NullPointerException.
Either the docs should be updated to reflect this change, or preferably, the old behavior should be reinstated. It would seem that many previously published SurfaceView apps would suffer from this.