We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 4206 - crash in SDLSurface.surfaceChanged()
Summary: crash in SDLSurface.surfaceChanged()
Status: RESOLVED FIXED
Alias: None
Product: SDL
Classification: Unclassified
Component: main (show other bugs)
Version: 2.0.4
Hardware: All Android (All)
: P2 normal
Assignee: Sam Lantinga
QA Contact: Sam Lantinga
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-06-23 09:50 UTC by Daniel Sobe
Modified: 2019-01-16 17:51 UTC (History)
1 user (show)

See Also:


Attachments
Java activity (71.62 KB, text/plain)
2018-07-04 05:34 UTC, Daniel Sobe
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Sobe 2018-06-23 09:50:45 UTC
I'm getting feedback on the Google Play Developer console about crashes of my app which uses SDL2, it is a NULL PTR in this line:

int requestedOrientation = SDLActivity.mSingleton.getRequestedOrientation();

in method

public void surfaceChanged(SurfaceHolder holder, int format, int width, int height)

Unfortunately I do not know under which circumstances it happens, and it doesn't occur on my devices.

I'd like to protect the line with a NULL PTR check so that it doesn't crash. But I have no idea what to do if the orientation cannot be read (w.r.t. the code that evaluates whether the "skip" variable is to be set to true or false).

I want to avoid to have the app "hang" due to incorrectly setting the "skip" flag. A crash is better I believe because the user sees that the app needs to be started again.
Comment 1 Sylvain 2018-07-02 11:05:42 UTC
Current, I don't think it can crash there, and I see nothing for this issue in my app logs, even with many users.

Do you have a stack trace ? maybe you have a too old SDL version and the bug as been fixed.

If this "skip" value is not set, you'll have a visual glitch / bad rendering (portrait vs landscape rendering)/
Comment 2 Daniel Sobe 2018-07-02 13:03:16 UTC
I have 3 reports in the play console.

2 of them are from Samsung devices:

Samsung Galaxy S7 (herolte), Android 7.0
Samsung Galaxy S6 Edge (zerolte), Android 7.0

Stacktrace:

java.lang.NullPointerException: 
 
  at org.libsdl.app.SDLSurface.surfaceChanged (SDLActivity.java:1400)
 
  at android.view.SurfaceView.updateWindow (SurfaceView.java:668)
 
  at android.view.SurfaceView.onWindowVisibilityChanged (SurfaceView.java:266)
 
  at android.view.View.dispatchWindowVisibilityChanged (View.java:11077)
 
  at android.view.ViewGroup.dispatchWindowVisibilityChanged (ViewGroup.java:1290)
 
  at android.view.ViewGroup.dispatchWindowVisibilityChanged (ViewGroup.java:1290)
 
  at android.view.ViewGroup.dispatchWindowVisibilityChanged (ViewGroup.java:1290)
 
  at android.view.ViewGroup.dispatchWindowVisibilityChanged (ViewGroup.java:1290)
 
  at android.view.ViewRootImpl.performTraversals (ViewRootImpl.java:1813)
 
  at android.view.ViewRootImpl.doTraversal (ViewRootImpl.java:1515)
 
  at android.view.ViewRootImpl$TraversalRunnable.run (ViewRootImpl.java:7091)
 
  at android.view.Choreographer$CallbackRecord.run (Choreographer.java:927)
 
  at android.view.Choreographer.doCallbacks (Choreographer.java:702)
 
  at android.view.Choreographer.doFrame (Choreographer.java:638)
 
  at android.view.Choreographer$FrameDisplayEventReceiver.run (Choreographer.java:913)
 
  at android.os.Handler.handleCallback (Handler.java:751)
 
  at android.os.Handler.dispatchMessage (Handler.java:95)
 
  at android.os.Looper.loop (Looper.java:154)
 
  at android.app.ActivityThread.main (ActivityThread.java:6682)
 
  at java.lang.reflect.Method.invoke (Native Method)
 
  at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run (ZygoteInit.java:1520)
 
  at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:1410)




The 3rd stack trace is from a Huawei P9 (HWEVA), Android 7.0




java.lang.NullPointerException: 
 
  at org.libsdl.app.SDLSurface.surfaceChanged (SDLActivity.java:1400)
 
  at android.view.SurfaceView.updateWindow (SurfaceView.java:664)
 
  at android.view.SurfaceView.setFrame (SurfaceView.java:334)
 
  at android.view.View.layout (View.java:17685)
 
  at android.widget.RelativeLayout.onLayout (RelativeLayout.java:1079)
 
  at android.view.View.layout (View.java:17688)
 
  at android.view.ViewGroup.layout (ViewGroup.java:5631)
 
  at android.widget.FrameLayout.layoutChildren (FrameLayout.java:325)
 
  at android.widget.FrameLayout.onLayout (FrameLayout.java:261)
 
  at android.view.View.layout (View.java:17688)
 
  at android.view.ViewGroup.layout (ViewGroup.java:5631)
 
  at android.widget.LinearLayout.setChildFrame (LinearLayout.java:1762)
 
  at android.widget.LinearLayout.layoutVertical (LinearLayout.java:1606)
 
  at android.widget.LinearLayout.onLayout (LinearLayout.java:1515)
 
  at android.view.View.layout (View.java:17688)
 
  at android.view.ViewGroup.layout (ViewGroup.java:5631)
 
  at android.widget.FrameLayout.layoutChildren (FrameLayout.java:325)
 
  at android.widget.FrameLayout.onLayout (FrameLayout.java:261)
 
  at com.android.internal.policy.DecorView.onLayout (DecorView.java:774)
 
  at android.view.View.layout (View.java:17688)
 
  at android.view.ViewGroup.layout (ViewGroup.java:5631)
 
  at android.view.ViewRootImpl.performLayout (ViewRootImpl.java:2513)
 
  at android.view.ViewRootImpl.performTraversals (ViewRootImpl.java:2228)
 
  at android.view.ViewRootImpl.doTraversal (ViewRootImpl.java:1366)
 
  at android.view.ViewRootImpl$TraversalRunnable.run (ViewRootImpl.java:6768)
 
  at android.view.Choreographer$CallbackRecord.run (Choreographer.java:926)
 
  at android.view.Choreographer.doCallbacks (Choreographer.java:735)
 
  at android.view.Choreographer.doFrame (Choreographer.java:667)
 
  at android.view.Choreographer$FrameDisplayEventReceiver.run (Choreographer.java:912)
 
  at android.os.Handler.handleCallback (Handler.java:761)
 
  at android.os.Handler.dispatchMessage (Handler.java:98)
 
  at android.os.Looper.loop (Looper.java:156)
 
  at android.app.ActivityThread.main (ActivityThread.java:6523)
 
  at java.lang.reflect.Method.invoke (Native Method)
 
  at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run (ZygoteInit.java:942)
 
  at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:832)
Comment 3 Daniel Sobe 2018-07-02 13:09:31 UTC
The SDL version I use is not the recent one, but I regularly look at the diff between my version which is around the time 2.0.5 was released, and the recent HG version. So I hope that I'm not missing important bug fixes. But of course I cannot say for sure.

Not being a frequently reported crash, I assume it will be hard to trigger (haven't managed myself). Might be that it only happens in certain situations of the Android app lifecycle, like e.g. resuming the app from the background. Somehow the system is calling onSurfaceChanged() before onCreate() which sounds pretty strange.
Comment 4 Sylvain 2018-07-02 18:55:14 UTC
Can you also post your SDLActivity.java ?
Comment 5 Daniel Sobe 2018-07-04 05:34:17 UTC
Created attachment 3268 [details]
Java activity

The modifications to the file are:

* Added some code for in-app-billing (most likely irrelevant)
* Added some code for checking whether to download and/or mount expansion files (most likely irrelevant as well)
* Added one patch to disable system bars/go to sticky immersive mode upon resume (that one might be interesting w.r.t. this bug, although I don't see anything wrong with it yet)
Comment 6 Sylvain 2018-07-04 17:16:01 UTC
The stack trace tells that mSingleton is null when calling 
"SDLActivity.mSingleton.getRequestedOrientation();"

And you initialized it in your method "onCreateNoDownload()". I don't know when/how it's called, it seems to be modification on your side. Maybe it's not called sometimes.
Comment 7 Daniel Sobe 2018-07-07 14:07:55 UTC
Yes, now that you mention it, I have overloaded SDLActivity and added all the code from the Android code example how to download missing expansion files. 

In that (unlikely when you've installed the app via Google Play) case, a different screen is shown until the download finishes.

Because I'm only overloading some of the lifecycle callbacks, indeed the surfaceChanged() call will land in SDLActivity, while the downloader is still in progress and SDL not even initalized :-(

Thanks for pointing this out!

As I cannot reproduce the bug itself, I assume that the explanation above is most likely correct and thus the bug not in SDL itself. Thus I'm marking it as resolved/invalid.
Comment 8 Sylvain 2018-07-07 14:39:00 UTC
ok. maybe there is no real issue, just the mSingleton beeing not set when taking some path.
Comment 9 Daniel Sobe 2018-07-08 10:26:11 UTC
One thing that I'm wondering though is whether there is still some sort of memory leak in SDLActivity.java (which could also explain the unexpected callback in my case, because my modifications probably make things worse).

What I'm wondering about are these lines:

        mSurface = new SDLSurface(getApplication());

        mLayout = new RelativeLayout(this);
        mLayout.addView(mSurface);

        setContentView(mLayout);
        
mSurface and mLayout are newly created upon every activity start. The resulting layout object is set as current view.

Because I did not find anything related in onDestroy() I'm wondering whether these objects will actually be fully dereferenced once the app is in this stage of the lifecycle. NULLing the objects references in SDLActivity.java alone probably won't help, because the Activity might still have a ref to mLayout, and mLayout definitely has a ref to mSurface. So during onCreate() both instances are recreated, but maybe the old ones are still active at that time.

In principle, subsequent calls to setContentView() should remove the last reference to older instances of mLayout. I'm not sure though the garbage collector will detect these though.

If my assumptions are correct, then it might be possible that onSurfaceChanged() can be called while the SDLActivity's singleton is not initialized (yet), because the callback happens to an older instance of SDLSurface from the previous apps lifecycle.

Nevertheless, with the regular version of SDLActivity I guess its still highly unlikely if not impossible to trigger this behaviour. If you think that all of this is too much of a theoretical issue, you may close the bug again.
Comment 10 Sylvain 2018-07-08 20:45:56 UTC
not sure, 
here's the life cycle diagram : https://developer.android.com/reference/android/app/Activity

I understand that after onDestroy(), everything should be released.

if it helps, my AndroidManifest.xml has also those lines :

android:alwaysRetainTaskState="true"
android:launchMode="singleInstance"
Comment 11 Sylvain 2019-01-16 15:08:20 UTC
Mark as fixed, only one instance is available:
https://hg.libsdl.org/SDL/rev/0ef0e4cb7752
Comment 12 Sylvain 2019-01-16 17:51:40 UTC
Mark as fixed