audio: threading and device hang improvements.
authorRyan C. Gordon
Fri, 07 Oct 2016 14:35:25 -0400
changeset 10471fd0bebfab510
parent 10470 7c2fad0ef5fd
child 10472 a3d50ebe2a3b
audio: threading and device hang improvements.

This tries to make SDL robust against device drivers that have hung up,
apps don't freeze in catastrophic (but not necessarily uncommon) conditions.

Now we detach the audio thread and let it clean up and don't care if it
never actually runs to completion.
src/audio/SDL_audio.c
src/audio/SDL_sysaudio.h
     1.1 --- a/src/audio/SDL_audio.c	Fri Oct 07 11:18:55 2016 -0700
     1.2 +++ b/src/audio/SDL_audio.c	Fri Oct 07 14:35:25 2016 -0400
     1.3 @@ -259,15 +259,28 @@
     1.4      }
     1.5  }
     1.6  
     1.7 +static void
     1.8 +SDL_AudioLockOrUnlockDeviceWithNoMixerLock(SDL_AudioDevice * device)
     1.9 +{
    1.10 +}
    1.11  
    1.12  static void
    1.13 -finalize_audio_entry_points(void)
    1.14 +finish_audio_entry_points_init(void)
    1.15  {
    1.16      /*
    1.17       * Fill in stub functions for unused driver entry points. This lets us
    1.18       *  blindly call them without having to check for validity first.
    1.19       */
    1.20  
    1.21 +    if (current_audio.impl.SkipMixerLock) {
    1.22 +        if (current_audio.impl.LockDevice == NULL) {
    1.23 +            current_audio.impl.LockDevice = SDL_AudioLockOrUnlockDeviceWithNoMixerLock;
    1.24 +        }
    1.25 +        if (current_audio.impl.UnlockDevice == NULL) {
    1.26 +            current_audio.impl.UnlockDevice = SDL_AudioLockOrUnlockDeviceWithNoMixerLock;
    1.27 +        }
    1.28 +    }
    1.29 +
    1.30  #define FILL_STUB(x) \
    1.31          if (current_audio.impl.x == NULL) { \
    1.32              current_audio.impl.x = SDL_Audio##x##_Default; \
    1.33 @@ -695,6 +708,35 @@
    1.34      free_audio_queue(packet);
    1.35  }
    1.36  
    1.37 +void
    1.38 +SDL_FinalizeAudioDevice(SDL_AudioDevice *device)
    1.39 +{
    1.40 +    if (!device) {
    1.41 +        return;
    1.42 +    }
    1.43 +
    1.44 +    /* lock/unlock here so we don't race if the audio thread saw the shutdown
    1.45 +       var without locking, and the thread that requested shutdown is now
    1.46 +       trying to unlock the mutex while we destroy it. Threading is hard. */
    1.47 +    current_audio.impl.LockDevice(device);
    1.48 +    current_audio.impl.UnlockDevice(device);
    1.49 +
    1.50 +    if (device->mixer_lock != NULL) {
    1.51 +        SDL_DestroyMutex(device->mixer_lock);
    1.52 +    }
    1.53 +    SDL_free(device->fake_stream);
    1.54 +    if (device->convert.needed) {
    1.55 +        SDL_free(device->convert.buf);
    1.56 +    }
    1.57 +    if (device->hidden != NULL) {
    1.58 +        current_audio.impl.CloseDevice(device);
    1.59 +    }
    1.60 +
    1.61 +    free_audio_queue(device->buffer_queue_head);
    1.62 +    free_audio_queue(device->buffer_queue_pool);
    1.63 +
    1.64 +    SDL_free(device);
    1.65 +}
    1.66  
    1.67  /* The general mixing thread function */
    1.68  static int SDLCALL
    1.69 @@ -773,6 +815,8 @@
    1.70      /* !!! FIXME: can we rename this WaitDrain? */
    1.71      current_audio.impl.WaitDone(device);
    1.72  
    1.73 +    SDL_FinalizeAudioDevice(device);
    1.74 +
    1.75      return 0;
    1.76  }
    1.77  
    1.78 @@ -854,6 +898,8 @@
    1.79  
    1.80      current_audio.impl.FlushCapture(device);
    1.81  
    1.82 +    SDL_FinalizeAudioDevice(device);
    1.83 +
    1.84      return 0;
    1.85  }
    1.86  
    1.87 @@ -949,7 +995,7 @@
    1.88  
    1.89      current_audio.detectionLock = SDL_CreateMutex();
    1.90  
    1.91 -    finalize_audio_entry_points();
    1.92 +    finish_audio_entry_points_init();
    1.93  
    1.94      /* Make sure we have a list of devices available at startup. */
    1.95      current_audio.impl.DetectDevices();
    1.96 @@ -1064,26 +1110,70 @@
    1.97  static void
    1.98  close_audio_device(SDL_AudioDevice * device)
    1.99  {
   1.100 -    SDL_AtomicSet(&device->shutdown, 1);
   1.101 -    SDL_AtomicSet(&device->enabled, 0);
   1.102 -    if (device->thread != NULL) {
   1.103 -        SDL_WaitThread(device->thread, NULL);
   1.104 +    if (!device) {
   1.105 +        return;
   1.106      }
   1.107 -    if (device->mixer_lock != NULL) {
   1.108 -        SDL_DestroyMutex(device->mixer_lock);
   1.109 -    }
   1.110 -    SDL_free(device->fake_stream);
   1.111 -    if (device->convert.needed) {
   1.112 -        SDL_free(device->convert.buf);
   1.113 -    }
   1.114 -    if (device->hidden != NULL) {
   1.115 -        current_audio.impl.CloseDevice(device);
   1.116 +
   1.117 +    /* It's possible the audio device can hang at the OS level for
   1.118 +       several reasons (buggy drivers, etc), so if we've got a thread in
   1.119 +       flight, we mark the device as ready to shutdown and return
   1.120 +       immediately. The thread will either notice this and clean everything
   1.121 +       up when it can, or it's frozen and helpless, but since we've already
   1.122 +       detached the thread, it's none of our concern. Otherwise, we might
   1.123 +       hang too. */
   1.124 +
   1.125 +    /* Note this can still hang if we initialized the device but failed
   1.126 +       to finish setting up, forcing _this_ thread to do the cleanup, but
   1.127 +       oh well. */
   1.128 +
   1.129 +    /* take it out of our open list now, though, even if device hangs. */
   1.130 +    if (device->id > 0) {
   1.131 +        SDL_AudioDevice *opendev = open_devices[device->id - 1];
   1.132 +        SDL_assert((opendev == device) || (opendev == NULL));
   1.133 +        if (opendev == device) {
   1.134 +            open_devices[device->id - 1] = NULL;
   1.135 +        }
   1.136      }
   1.137  
   1.138 -    free_audio_queue(device->buffer_queue_head);
   1.139 -    free_audio_queue(device->buffer_queue_pool);
   1.140 +    if (!current_audio.impl.ProvidesOwnCallbackThread && !device->thread) {
   1.141 +        /* no thread...maybe we're cleaning up a half-opened failure. */
   1.142 +        SDL_FinalizeAudioDevice(device); /* do it ourselves. */
   1.143 +    } else if (current_audio.impl.ProvidesOwnCallbackThread) {
   1.144 +        /* !!! FIXME: this is sort of a mess, because we _should_ treat this
   1.145 +           !!! FIXME:  the same as our internal threads, but those targets
   1.146 +           !!! FIXME:  need refactoring first: they need to call
   1.147 +           !!! FIXME:  SDL_FinalizeAudioDevice() themselves and also have
   1.148 +           !!! FIXME:  their CloseDevice() code deal with cleaning up
   1.149 +           !!! FIXME:  half-initialized opens _and_ normal runs. So for now,
   1.150 +           !!! FIXME:  nothing to do here but pray this doesn't hang.
   1.151 +           !!! FIXME:  (the TODO list: coreaudio, emscripten, nacl, haiku) */
   1.152 +        SDL_FinalizeAudioDevice(device);
   1.153 +    } else {
   1.154 +        Uint32 delay = 0;
   1.155  
   1.156 -    SDL_free(device);
   1.157 +        if (!device->iscapture) {
   1.158 +            /* x2000: x1000 (cvt to ms) and x2 (alternating DMA buffers). */
   1.159 +            const SDL_AudioSpec *spec = &device->spec;
   1.160 +            delay = (Uint32) ((((float)spec->samples) / ((float)spec->freq)) * 2000.0f);
   1.161 +        }
   1.162 +
   1.163 +        /* Lock to make sure an audio callback doesn't fire after we return.
   1.164 +           Presumably, if a device hangs, it'll not be holding this mutex,
   1.165 +           since it should only be held during the app's audio callback. */
   1.166 +        current_audio.impl.LockDevice(device);
   1.167 +        SDL_AtomicSet(&device->shutdown, 1);  /* let the thread do it. */
   1.168 +        SDL_AtomicSet(&device->enabled, 0);
   1.169 +        current_audio.impl.UnlockDevice(device);
   1.170 +
   1.171 +        /* device is no longer safe to touch at this point. */
   1.172 +
   1.173 +        if (delay > 0) {
   1.174 +            /* Block the amount that is roughly pending for playback, so we
   1.175 +               don't drop audio if the process exits right after this call. */
   1.176 +printf("audio close: delay for %u ms\n", (unsigned int) delay);
   1.177 +            SDL_Delay(delay);
   1.178 +        }
   1.179 +    }
   1.180  }
   1.181  
   1.182  
   1.183 @@ -1172,6 +1262,7 @@
   1.184          return 0;
   1.185      }
   1.186  
   1.187 +    /* !!! FIXME: there is a race condition here if two devices open from two threads at once. */
   1.188      /* Find an available device ID... */
   1.189      for (id = min_id - 1; id < SDL_arraysize(open_devices); id++) {
   1.190          if (open_devices[id] == NULL) {
   1.191 @@ -1397,10 +1488,13 @@
   1.192          device->thread = SDL_CreateThreadInternal(iscapture ? SDL_CaptureAudio : SDL_RunAudio, threadname, stacksize, device);
   1.193  
   1.194          if (device->thread == NULL) {
   1.195 -            SDL_CloseAudioDevice(device->id);
   1.196 +            close_audio_device(device);
   1.197              SDL_SetError("Couldn't create audio thread");
   1.198              return 0;
   1.199          }
   1.200 +
   1.201 +        /* don't ever join on this thread; it will clean itself up. */
   1.202 +        SDL_DetachThread(device->thread);
   1.203      }
   1.204  
   1.205      return device->id;
   1.206 @@ -1520,11 +1614,7 @@
   1.207  void
   1.208  SDL_CloseAudioDevice(SDL_AudioDeviceID devid)
   1.209  {
   1.210 -    SDL_AudioDevice *device = get_audio_device(devid);
   1.211 -    if (device) {
   1.212 -        close_audio_device(device);
   1.213 -        open_devices[devid - 1] = NULL;
   1.214 -    }
   1.215 +    close_audio_device(get_audio_device(devid));
   1.216  }
   1.217  
   1.218  void
   1.219 @@ -1543,9 +1633,7 @@
   1.220      }
   1.221  
   1.222      for (i = 0; i < SDL_arraysize(open_devices); i++) {
   1.223 -        if (open_devices[i] != NULL) {
   1.224 -            SDL_CloseAudioDevice(i+1);
   1.225 -        }
   1.226 +        close_audio_device(open_devices[i]);
   1.227      }
   1.228  
   1.229      free_device_list(&current_audio.outputDevices, &current_audio.outputDeviceCount);
     2.1 --- a/src/audio/SDL_sysaudio.h	Fri Oct 07 11:18:55 2016 -0700
     2.2 +++ b/src/audio/SDL_sysaudio.h	Fri Oct 07 14:35:25 2016 -0400
     2.3 @@ -93,7 +93,7 @@
     2.4      /* Some flags to push duplicate code into the core and reduce #ifdefs. */
     2.5      /* !!! FIXME: these should be SDL_bool */
     2.6      int ProvidesOwnCallbackThread;
     2.7 -    int SkipMixerLock;  /* !!! FIXME: do we need this anymore? */
     2.8 +    int SkipMixerLock;
     2.9      int HasCaptureSupport;
    2.10      int OnlyHasDefaultOutputDevice;
    2.11      int OnlyHasDefaultCaptureDevice;