| Summary: | Suspending app twice causes crash. | ||
|---|---|---|---|
| Product: | SDL | Reporter: | Jeremy Jurksztowicz <jurksztowicz> |
| Component: | video | Assignee: | Ryan C. Gordon <icculus> |
| Status: | RESOLVED FIXED | QA Contact: | Sam Lantinga <slouken> |
| Severity: | major | ||
| Priority: | P2 | CC: | armin.ronacher, icculus, tim, vitto.giova |
| Version: | HG 2.0 | ||
| Hardware: | iPhone/iPod touch | ||
| OS: | iOS (All) | ||
| Attachments: |
Possible patch for UIScreenMode being NULL
adding the required release and null for the new uiscreenmode Patch for more retain/release tapdancing. Patch for more retain/release tapdancing, attempt #2. |
||
|
Description
Jeremy Jurksztowicz
2011-04-27 17:54:48 UTC
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. 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? Created attachment 638 [details]
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.
*** Bug 1193 has been marked as a duplicate of this bug. *** The patch seems to do the right thing to me... Fixed in hg changeset dd0f52bf2bfa, thanks! --ryan. 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. Created attachment 690 [details]
adding the required release and null for the new uiscreenmode
Created attachment 714 [details] 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. Created attachment 715 [details] 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. (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. |