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 1579

Summary: Creating a texture with unsupported format may cause double-destruction
Product: SDL Reporter: Alexander Hirsch <1zeeky>
Component: videoAssignee: Sam Lantinga <slouken>
Status: RESOLVED FIXED QA Contact: Sam Lantinga <slouken>
Severity: minor    
Priority: P2 CC: 1zeeky
Version: HG 2.0   
Hardware: All   
OS: All   
Attachments: testcase writing the error to STDOUT
switch texture with texture->native in render->textures

Description Alexander Hirsch 2012-08-25 20:01:29 UTC
Created attachment 934 [details]
testcase writing the error to STDOUT

When creating a SDL_Texture with unsupported format (I'll now refer to it as texture A), SDL_CreateTexture will call SDL_CreateTexture again with GetClosestSupportedFormat to set texture->native (which I will now refer to as texture B).
This causes texture B to be put before A in renderer->textures.

If texture A is explicitly destroyed, everything is fine. Otherwise, upon SDL_DestroyRenderer, the loop will first encounter texture B, destroy it, then texture A, destroy that which will want to destroy texture->native and since it is already destroyed set an error.

The solution could be as simple as swapping texture A with B after texture->native gets set in SDL_CreateTextures.
A different solution that would not change the order would be to destroy the textures in reverse order, but that would either require another pointer to the end of the textures-list or double the number of iterations to find the end first.
Perhaps the "native texture" could check if it gets referenced by the next texture in the list, but that does not sound like a pretty solution.

I've attached a small test-case demonstrating the error by doing as said using the software renderer and the unsupported format SDL_PIXELFORMAT_RGB24 and printing the error to STDOUT.
Comment 1 Alexander Hirsch 2012-08-25 20:05:01 UTC
Created attachment 935 [details]
switch texture with texture->native in render->textures

I looked if changing the order of the textures would have any further impact, but the list seems to be used solely to clean up if someone did not destroy the textures themselves, thus I've attached the patch that will switch the order of the texture and its texture->native.
This will destroy the "non-native" texture first, which destroys texture->native as well, both are unlinked from the list and so the native texture will not be destroyed again.
Comment 2 Sam Lantinga 2012-09-28 04:10:04 UTC
Fixed, thanks!
http://hg.libsdl.org/SDL/rev/e844e2632149