Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suspending app twice causes crash. #407

Closed
SDLBugzilla opened this issue Feb 10, 2021 · 0 comments
Closed

Suspending app twice causes crash. #407

SDLBugzilla opened this issue Feb 10, 2021 · 0 comments

Comments

@SDLBugzilla
Copy link
Collaborator

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

Reported in version: HG 2.0
Reported for operating system, platform: iOS (All), iPhone/iPod touch

Comments on the original bug report:

On 2011-04-27 17:54:48 +0000, Jeremy Jurksztowicz wrote:

In the UIKit_AddDisplay function in SDL_uikitvideo.m, you create desktop_mode and current_mode SDL_DisplayMode instances without assigning them driver data. Later on UIKit_SetDisplayMode expects them to have valid driver data.

POSSIBLE SOLUTION:
Change function [void UIKit_AddDisplay(UIScreen *uiscreen, int w, int h)]
to [void UIKit_AddDisplay(UIScreen *uiscreen, UIScreenMode *uimode, int w, int h)]
and assign the retained uimode to the respective driverdatas.

On 2011-05-08 00:54:03 +0000, Armin Ronacher wrote:

Shouldn't the modes be set on video mode enumeration on the structure? If yes, one could just trigger the execution of this code once on startup and the problem should go away. In case this solves the problem the proper solution would probably be to ensure that both SetVideoMode and the enumeration functions calls into the same piece of code that modify these structures.

On 2011-05-08 07:43:03 +0000, Jeremy Jurksztowicz wrote:

Not sure if I get what you mean, but...
UIKit_AddDisplay is only called by UIKit_VideoInit, so the modes are created and assigned to driverdata once.
Maybe I misunderstood your suggestion?

On 2011-06-27 15:10:50 +0000, Mike Flynn wrote:

Created attachment 638
Possible patch for UIScreenMode being NULL

This is a possible patch for the problem reported here and in 1193.

I'm not really sure how this process goes, I thought I would just offer a patch. Let me know if I should be doing something else to help get this bug fixed.

On 2011-06-30 09:09:02 +0000, Tim Angus wrote:

*** Bug 1193 has been marked as a duplicate of this bug. ***

On 2011-06-30 09:18:07 +0000, Tim Angus wrote:

The patch seems to do the right thing to me...

On 2011-07-05 19:07:28 +0000, Ryan C. Gordon wrote:

Fixed in hg changeset dd0f52bf2bfa, thanks!

--ryan.

On 2011-08-28 02:32:12 +0000, Vittorio Giovara wrote:

I have to reopen this bugreport because the current fix is causing a double free error; this is normally unnoticable because when the last sdl window closes also the app is shut down, but in case sdl init/quit is called multiple times the doublefree error creates all sorts of havoc, like nsautoreleasepool being corrupted, cfrelease to object that are not supposed to be released and exeptions for unrecognised selector.

The problem lies in how UIKit_VideoQuit and SDL_VideoQuit interoperate, that is all the driverdata is released and then NULL'd in the latter so that in the former it isn't freed: however since from now on we care about uiscreenmodes, the driver data of one of the modes is not freed'n'nulled, and so the uiscreenmode gets freed instead of released. As you can guess this is nasty.

I'm attaching a patch that builds on dd0f52bf2bfa and adds the required checks for taking care of the uiscreenmode.

Also dd0f52bf2bfa raises the minimum ios version to 3.2, I believe the xcodeproject should be updated to reflect that.

On 2011-08-28 02:33:03 +0000, Vittorio Giovara wrote:

Created attachment 690
adding the required release and null for the new uiscreenmode

On 2011-10-14 17:34:54 +0000, Ryan C. Gordon wrote:

Created attachment 714
Patch for more retain/release tapdancing.

(In reply to comment # 8)

Created attachment 690 [details]
adding the required release and null for the new uiscreenmode

This patch doesn't appear to be entirely correct...we retain this object once (and then use the pointer in two places), but might overwrite the pointer elsewhere when changing video modes, so I've updated this patch to handle those cases.

This is untested, though.

(Also, I should clean up the desktop_mode vs current_mode FIXME, but let's do that after this bug is resolved.)

--ryan.

On 2011-10-14 22:53:01 +0000, Ryan C. Gordon wrote:

Created attachment 715
Patch for more retain/release tapdancing, attempt # 2.

(In reply to comment # 9)

(Also, I should clean up the desktop_mode vs current_mode FIXME, but let's do
that after this bug is resolved.)

Actually, that part wasn't hard: attaching updated patch!

This compiles for iOS, but I haven't really tried it. Barring objections, I'll probably push this patch tomorrow.

--ryan.

On 2011-10-16 00:07:11 +0000, Ryan C. Gordon wrote:

(In reply to comment # 10)

This compiles for iOS, but I haven't really tried it. Barring objections, I'll
probably push this patch tomorrow.

This is now hg changeset 69875bbf83d8.

--ryan.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant