libsdl-org / sdl12-compat

An SDL-1.2 compatibility layer that uses SDL 2.0 behind the scenes.
Other
193 stars 40 forks source link

perl-SDL testsuite get stuck in SDL_FreeYUVOverlay() after SDL_LockMutex_REAL() since 1.2.56 (1.2.52 was OK) #275

Closed soig closed 1 year ago

soig commented 1 year ago

It builds smoothly with 1.2.52 : See logs https://pkgsubmit.mageia.org/autobuild/cauldron/x86_64/core/2022-08-19/perl-SDL-2.548.0-10.mga9.src.rpm/

But it but testsuite got stucks with 1.2.56 & 1.2.60 on both Fedora & Mageia distributions: Here's the logs with 1.2.56: https://pkgsubmit.mageia.org/autobuild/cauldron/x86_64/core/2022-10-31/perl-SDL-2.548.0-10.mga9.src.rpm/)

Running it manually shows that it's stuck in SDL_FreeYUVOverlay(). I've attached the GDB backtraces for both Fedora & Mageia distributions.. bt-mga.txt bt-fc.txt

So 1.2.56 broke down (as 1.2.54 was the same but a pre-version?). Downgrading to 1.2.52 immediately fix (no more hang)

How to reproduce:

lftpget https://cpan.metacpan.org/authors/id/F/FR/FROGGS/SDL-2.548.tar.gz
tar xf SDL-2.548.tar.gz
cd SDL-2.548/
perl Build.PL 
./Build
# Then you can either run ./Build test or just run the offending test:
perl -Iblib/lib -Iblib/arch t/core_video.t 
# or just run it directly in gdb ("break SDL_FreeYUVOverlay" then "r"):
gdb -q --args perl -Iblib/lib -Iblib/arch t/core_video.t

It seems to hang right after playing with some mutex (using SDL_LockMutex_REAL())

soig commented 1 year ago

This might be the offending commit: https://github.com/libsdl-org/sdl12-compat/commit/184565a3ae07cbcc508d8427b51b5b5b3ca01f05

slouken commented 1 year ago

Can you attach the backtrace from all the threads in the process?

soig commented 1 year ago

"thread apply all bt" just show only one thread with the same trace.

soig commented 1 year ago

Just install the perl-SDL package from your distribution and run the minimal test script I'm attaching. I'm also attaching the backtrace from all the threads. bt-fc2.txt gdb -q --args perl /tmp/stuck.txt stuck.txt

This script is just the minimal stripped down version of perl-SDL's t/core_video.t test file (https://metacpan.org/release/FROGGS/SDL-2.548/source/t/core_video.t). I've added two "if(1)" sections, changing the inner one to "if(0)" makes the bug to disappear.

slouken commented 1 year ago

Oh yeah, that's clearly wrong.

Here, try this:

diff --git a/src/SDL12_compat.c b/src/SDL12_compat.c
index 6964e2b..fb657a6 100644
--- a/src/SDL12_compat.c
+++ b/src/SDL12_compat.c
@@ -7535,6 +7535,7 @@ SDL_FreeYUVOverlay(SDL12_Overlay *overlay12)
             if (overlay->overlay12 == overlay12) {
                 overlay->overlay12 = NULL;  /* don't try to draw this later. */
             }
+            overlay = overlay->next;
         }

         if (renderer) {
soig commented 1 year ago

That fixed it on the Mageia box

slouken commented 1 year ago

Great, thanks!

icculus commented 1 year ago

Good catch, Sam, thanks!