SDL_SemWaitTimeout in src/thread/generic/SDL_syssem.c line 179 (SVN trunk):
--sem->count;
should be
if (retval == 0) {
--sem->count;
}
Without this, sem->count will underflow on timeout effectively breaking the
semaphore. It appears that the implementation has been wrong since the initial
revision.
Betreff: [SDL] [PATCH] Make static variables const
Datum: Tue, 19 May 2009 19:45:37 +0200
Hi,
this is a set of simple changes which make some of SDL's internal static
arrays constant. The purpose is to shrink the number of write-able
static bytes and thus increase the number of memory pages shared between
SDL applications.
The patch set is against trunk@4513. Each of the attached patch files is
specific to a sub-system. The set is completed by a second mail, because
of the list's 40 KiB limit.
The files readelf-r4513.txt and readelf-const-patch.txt where made by
calling 'readelf -S libSDL.so'. They show the difference in ELF sections
without and with the patch. Some numbers measured on my x86-64:
Before
[13] .rodata PROGBITS 00000000000eaaa0 000eaaa0
0000000000008170 0000000000000000 A 0 0 32
[19] .data.rel.ro PROGBITS 00000000003045e0 001045e0
00000000000023d0 0000000000000000 WA 0 0 32
[23] .data PROGBITS 00000000003076e0 001076e0
0000000000004988 0000000000000000 WA 0 0 32
After
[13] .rodata PROGBITS 00000000000eaaa0 000eaaa0
0000000000009a50 0000000000000000 A 0 0 32
[19] .data.rel.ro PROGBITS 0000000000306040 00106040
0000000000002608 0000000000000000 WA 0 0 32
[23] .data PROGBITS 0000000000309360 00109360
0000000000002e88 0000000000000000 WA 0 0 32
The size of the write-able data section decreased considerably. Some
entries became const-after-relocation, while most of its content went
straight into the read-only data section.
Best regards, Thomas
From: John Bartholomew
Subject: [SDL] SDL Semaphore implementation broken on Windows?
Hi,
Over the past couple of days, I've been battling with SDL, SDL_Mixer and SMPEG to try to find an audio hang bug. I believe I've found the problem, which I think is a race condition inside SDL's semaphore implementation (at least the Windows implementation). The semaphore code uses Windows' built in semaphore functions, but it also maintains a separate count value. This count value is updated with bare increment and decrement operations in SemPost and SemWaitTimeout - no locking primitives to protect them.
In tracking down the apparent audio bug, I found that at some point a semaphore's count value was being decremented to -1, which is clearly not a valid value for it to take.
I'm still not certain exactly what sequence of operations is occuring for this to happen, but I believe that overall it's a race condition between a thread calling SemPost (which increments the count) and the thread on the other end calling SemWait (which decrements it).
I will try to make a test case to verify this, but I'm not sure if I'll be able to (threading errors being difficult to reproduce even in the best circumstances).
However, assuming this is the cause of my problems, there is a very
simple fix:
Windows provides InterlockedIncrement() and InterlockedDecrement()
functions to perform increments and decrements which are guaranteed to be atomic. So the fix is in thread/win32/SDL_syssem.c: replace occurrences of --sem->count with InterlockedDecrement(&sem->count); and replace occurrences of ++sem->count with InterlockedIncrement(&sem->count);
This is using SDL v1.2.12, built with VC++ 2008 Express, running on a
Core 2 duo processor.
The current SVN trunk is missing the SDLCALL specifier at numerous locations.
It has to be added for all (possibly user provided) callbacks.
I stumbled over this while creating a makefile for the OpenWatcom compiler for
Win32.
SDL_SemValue() always returns 0. That's because the underlying POSIX
sem_getvalue() is implemented as a noop in Mac OS X. Apart from that,
semaphores do work properly (at least according to test/testsem), so I'm not
sure if this is worth fixing at all.
From: "alan buckley"
Subject: Re: Updating SDL for RISC OS
I've attached a zip file with three diffs to get SDL to build on RISC OS.
configure.in.diff and makedeps.sh.diff have been modified so that they
can build the assembler file in video/riscos. You may want to double
check these as I don't know a lot about unix shell programming.
(I've tested them cross-compiling on cygwin).
sdlsysthread.diff is the changes required to SDL_systhread.c for RISC
OS. As you suspected with these changes in place you can delete the
RISC OS specific threading code in thread/riscos.
I've also renamed SDL_riscosASM.s to SDL_riscosASM.S, but I'm never
quite sure how to rename files in CVS, so if you could to this it would
be appreciated.
FIXME:
Change #include <stdlib.h> to #include "SDL_stdlib.h"
Change #include <string.h> to #include "SDL_string.h"
Make sure nothing else broke because of this...
From: Jon Daniel <forcemaster@gmx.net>
To: sdl@libsdl.org
Subject: [SDL] SDL_SemWaitTimeout patch for BeOS
The current SDL-1.2.9/src/thread/beos/SDL_syssem.c returns -1 if the
acquire_sem timeouts which according to the DocWiki is not correct.
I've added support for the B_TIMED_OUT and B_WOULD_BLOCK in the switch.
Jon Daniel
From: "alan buckley"
Subject: SDL Improved semiphore implementation for RISC OS (version 2)
I've attached a new version of the patch for the RISC OS
semaphore processing (in a zip file) that updates it to use
the improved semaphores support in UnixLiib.
From: Peter Naulls
Subject: RISC OS SDL Patches
Sam, I've attached a diff of the latest changes to libSDL for RISC OS
support. These changes are by Alan Buckley and myself.
The most significant of these are:
Optimised assembler blit rountines - I've attached the file
src/video/riscos/SDL_riscosASM.s which is needed for this.
Move to using /dev/dsp instead of its own audio implementation.
This means that src/audio/riscos/SDL_drenderer.c should be removed
Typo fixes. Mainly correct spelling of "RISC OS", but some from elsewhere
too.
From: "alan buckley"
Subject: Modification for RISC OS version of SDL
Ive attached a zip file with the changes to this email, it contains the
following:
The file sdldiff.txt is the output from cvs diff u. .
The directory thread/riscos contains all the new files to support threading.
Readme.riscos is a new readme file to add.
