Concerning removal of timer callbacks in SDL2, ie. 'SDL_RemoveTimer()'

Hi list,

I’ve recently come about a peculiar detail of the SDL2 timer callback
interface, specifically in regards
to removing active timer callbacks. Imagine following C++ scenario: you
have a class with certain
state, and schedule an SDL timer to callback into one of this classes
function periodically doing
some task (eg. pumping audio buffers). When the class object is destroyed,
you want to clean up
the timer callback to make sure that it isn’t called anymore after the
object’s lifetime has ended so
it doesn’t access/modify at that point undefined memory.

Currently, there’s no way to do that. Looking at SDL2’s source, there seems
absolutely no
guarantee that the timer callback won’t be called one last time some time
after SDL_RemoveTimer
has already returned (and the containing object may have already left its
destructor). The only
lock I see in the source is a spinlock which is used to guarantee the
global timer callback list’s
integrity, but that’s about it. It seems like only a flag is set to
indicate “if you see this, please
consider stopping emission of the callback sometime in the future, mkay?”.

Is this intended? It seems like a pretty big design flaw in my view. Any
opinions / suggestions?
As for my code, I will most likely have to fallback to using threads
directly again (which does have
other benefits, so I’m not entirely stranded or anything).

Thanks,
Jonas

Hmm, seems like no replies on this. I have a crash which is happening because a timer is detroyed between when it is pulled from the queue for calling and when it gets called. I am surprised to see no-one else has reported this issue. Anyway, this seems to do the trick:


Index: third_party/SDL2-2.0.5/src/timer/SDL_timer.c

— third_party/SDL2-2.0.5/src/timer/SDL_timer.c (revision 94)
+++ third_party/SDL2-2.0.5/src/timer/SDL_timer.c (working copy)
@@ -159,13 +159,13 @@

     /* We're going to do something with this timer */
     data->timers = current->next;
  •   	SDL_LockMutex(data->timermap_lock);
       if (SDL_AtomicGet(&current->canceled)) {
           interval = 0;
       } else {
           interval = current->callback(current->interval, current->param);
       }
    
  •   	SDL_UnlockMutex(data->timermap_lock);
       if (interval > 0) {
           /* Reschedule this timer */
           current->scheduled = tick + interval;
    

Best,

Dave