SDL thread join frees first some data and later a thread tries to read same data

[SDLBugzilla]

This bug report was migrated from our old Bugzilla tracker.

Reported in version: 1.2.13
Reported for operating system, platform: Linux, x86

Comments on the original bug report:

On 2008-07-01 15:21:37 +0000, Pauli Nieminen wrote:

Valgrind reports memory write after free in SDL thread code:
==28062== Thread 2:
==28062== Invalid write of size 4
==28062== at 0x405F1AE: (within /usr/lib/libSDL-1.2.so.0.11.1)
==28062== by 0x40ABBBC: (within /usr/lib/libSDL-1.2.so.0.11.1)
==28062== by 0x43C8FD9: start_thread (pthread_create.c:297)
==28062== by 0x42FC93D: clone (in /usr/lib/debug/libc-2.7.so)
==28062== Address 0x4583a88 is 8 bytes inside a block of size 792 free'd
==28062== at 0x402165C: free (vg_replace_malloc.c:323)
==28062== by 0x405F303: SDL_WaitThread (in /usr/lib/libSDL-1.2.so.0.11.1)
==28062== by 0x807011D: threading::thread::join() (thread.cpp:74)
==28062== by 0x807013A: threading::thread::~thread() (thread.cpp:60)
==28062== by 0x8080CD5: (anonymous namespace)::process_queue(void*) (network_worker.cpp:598)
==28062== by 0x405F1AA: (within /usr/lib/libSDL-1.2.so.0.11.1)
==28062== by 0x40ABBBC: (within /usr/lib/libSDL-1.2.so.0.11.1)
==28062== by 0x43C8FD9: start_thread (pthread_create.c:297)
==28062== by 0x42FC93D: clone (in /usr/lib/debug/libc-2.7.so)

Here is websvn links to code which was used to generate valgrind report:
http://svn.gna.org/viewcvs/wesnoth/trunk/src/network_worker.cpp?rev=27624&view=markup
http://svn.gna.org/viewcvs/wesnoth/trunk/src/thread.hpp?rev=23842&view=markup
http://svn.gna.org/viewcvs/wesnoth/trunk/src/thread.cpp?rev=27646&view=markup

This report can be easily reproduced using svn trunk unit tests:
scons test build=debug
valgrind ./test-debug

I'm suspecting that this valgrind error might be source for some random crashes in wesnoth.

Athlon XP-M processor.
ubuntu hardy 8.04.
SDL 1.2.13 binary package.

On 2009-09-13 16:33:23 +0000, Ryan C. Gordon wrote:

Tagging this bug with "target-1.2.14" so we can try to resolve it for SDL 1.2.14.

Please note that we may choose to resolve it as WONTFIX. This tag is largely so we have a comprehensive wishlist of bugs to examine for 1.2.14 (and so we can close bugs that we'll never fix, rather than have them live forever in Bugzilla).

--ryan.

On 2009-09-21 01:51:33 +0000, Sam Lantinga wrote:

Ryan, can you look at this for 1.2 and 1.3? Thanks!

On 2009-09-22 13:54:34 +0000, Rene Dudfield wrote:

hello,

I think a fix might be for SDL_ThreadsQuit to use the thread_lock like the rest of the functions.

Here's what I think is the problem...

It's possible for a SDL_DelThread to put the threads in a state where SDL_ThreadsQuit is called(after SDL_DelThread has released thread_lock). However SDL_ThreadsQuit does not use thread_lock, which means another thread being created would have weird issues happening.

Imagine this sequence:

SDL_DelThread (acquires lock and gets to work)
SDL_AddThread (waits for lock)
SDL_DelThread (almost finishes work, and releases lock)
SDL_AddThread (acquires lock and gets to work)
SDL_DelThread (calls SDL_ThreadsQuit whilst AddThread is going)
SDL_ThreadsQuit (sets thread_lock to NULL)
... weirdness, where multiple threads can mess with the data structures.

cheers,

On 2009-09-22 23:38:24 +0000, Sam Lantinga wrote:

This might be fixed by the latest checkin:
http://www.libsdl.org/tmp/SDL-1.2.zip

Can you try it out and let me know if that takes care of the issue?

Thanks!

On 2009-10-10 01:13:39 +0000, Sam Lantinga wrote:

Tossing to Ryan for verification.

On 2009-10-10 09:54:44 +0000, Ryan C. Gordon wrote:

I couldn't trigger this with the wesnoth unit tests, even testing with a revision before the fix.

It would be useful if Pauli could retest and let us know if the issue is resolved.

--ryan.

On 2009-10-21 20:44:07 +0000, Sam Lantinga wrote:

E-mail to the author of this bug is bouncing. I believe this is fixed so I'm going to close this bug. Please re-open it if it's still active in SDL 1.3!