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 4451 - [patch] Loading GIFs on multiple threads returns garbage output and crashes
Summary: [patch] Loading GIFs on multiple threads returns garbage output and crashes
Status: RESOLVED FIXED
Alias: None
Product: SDL_image
Classification: Unclassified
Component: misc (show other bugs)
Version: 2.0.4
Hardware: x86_64 Windows 10
: P2 normal
Assignee: Sam Lantinga
QA Contact: Sam Lantinga
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-01-10 22:22 UTC by David Lönnhager
Modified: 2019-06-19 13:51 UTC (History)
1 user (show)

See Also:

Attachments
Load GIFs on multiple threads (15.63 KB, patch)
2019-01-10 22:22 UTC, David Lönnhager 		Details | Diff
fix access violation (545 bytes, patch)
2019-06-18 16:35 UTC, Dmitry Gapkalov 		Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description David Lönnhager 2019-01-10 22:22:56 UTC 
Created attachment 3565 [details]
Load GIFs on multiple threads

It's due to the use of global variables and static local variables. A quick fix was moving all of that into a "state" struct and declaring it in IMG_LoadGIF_RW(), which apparently fixed the issue.

The left image shows what the output might have looked like before the patch: https://i.imgur.com/bs26rG8.png

Loading PNGs, JPGs, and BMPs on multiple threads seems to work.
Comment 1 Sam Lantinga 2019-06-10 23:32:58 UTC 
Fixed, thanks!
https://hg.libsdl.org/SDL_image/rev/07fe7a23da89
Comment 2 Dmitry Gapkalov 2019-06-18 16:28:43 UTC 
Hello, I have a crash with this changes. Crash happens in:

    state->buf[0] = state->buf[state->last_byte - 2];   // access violation becasu in last_byte - trash
    state->buf[1] = state->buf[state->last_byte - 1];

    if ((count = GetDataBlock(src, &state->buf[2], state)) <= 0)
        state->done = TRUE;

    state->last_byte = 2 + count; // last_byte  initialization
Comment 3 Dmitry Gapkalov 2019-06-18 16:35:34 UTC 
Created attachment 3829 [details]
fix access violation

patch to fix access violation
Comment 4 Sam Lantinga 2019-06-19 13:51:54 UTC 
Patch added, thanks!
https://hg.libsdl.org/SDL_image/rev/02e1f7b5746c