[PATCH 2/5] winepulse: Use a critical section for PE-side locking.

[Jacek Caban]

Signed-off-by: Jacek Caban <jacek(a)codeweavers.com>
---
  dlls/winepulse.drv/mmdevdrv.c | 63 ++++++++++++++++++++---------------
  dlls/winepulse.drv/pulse.c    | 33 ++++++++++++------
  dlls/winepulse.drv/unixlib.h  |  2 --
  3 files changed, 58 insertions(+), 40 deletions(-)

[Andrew Eikum]

Signed-off-by: Andrew Eikum <aeikum(a)codeweavers.com>

On Wed, May 26, 2021 at 04:18:43PM +0200, Jacek Caban wrote:
> Signed-off-by: Jacek Caban <jacek(a)codeweavers.com>
> ---
>  dlls/winepulse.drv/mmdevdrv.c | 63 ++++++++++++++++++++---------------
>  dlls/winepulse.drv/pulse.c    | 33 ++++++++++++------
>  dlls/winepulse.drv/unixlib.h  |  2 --
>  3 files changed, 58 insertions(+), 40 deletions(-)

> diff --git a/dlls/winepulse.drv/mmdevdrv.c b/dlls/winepulse.drv/mmdevdrv.c
> index 1a39db0f72b..0350cb52e8c 100644
> --- a/dlls/winepulse.drv/mmdevdrv.c
> +++ b/dlls/winepulse.drv/mmdevdrv.c
> @@ -68,6 +68,15 @@ static GUID pulse_render_guid =
>  static GUID pulse_capture_guid =
>  { 0x25da76d0, 0x033c, 0x4235, { 0x90, 0x02, 0x19, 0xf4, 0x88, 0x94, 0xac, 0x6f } };
>
> +static CRITICAL_SECTION session_cs;
> +static CRITICAL_SECTION_DEBUG session_cs_debug = {
> +    0, 0, &session_cs,
> +    { &session_cs_debug.ProcessLocksList,
> +      &session_cs_debug.ProcessLocksList },
> +    0, 0, { (DWORD_PTR)(__FILE__ ": session_cs") }
> +};
> +static CRITICAL_SECTION session_cs = { &session_cs_debug, -1, 0, 0, 0, 0 };
> +
>  BOOL WINAPI DllMain(HINSTANCE dll, DWORD reason, void *reserved)
>  {
>      if (reason == DLL_PROCESS_ATTACH) {
> @@ -367,9 +376,9 @@ static ULONG WINAPI AudioClient_Release(IAudioClient3 *iface)
>          if (This->pulse_stream) {
>              pulse->release_stream(This->pulse_stream, This->timer);
>              This->pulse_stream = NULL;
> -            pulse->lock();
> +            EnterCriticalSection(&session_cs);
>              list_remove(&This->entry);
> -            pulse->unlock();
> +            LeaveCriticalSection(&session_cs);
>          }
>          IUnknown_Release(This->marshal);
>          IMMDevice_Release(This->parent);
> @@ -549,10 +558,10 @@ static HRESULT WINAPI AudioClient_Initialize(IAudioClient3 *iface,
>          return E_INVALIDARG;
>      }
>
> -    pulse->lock();
> +    EnterCriticalSection(&session_cs);
>
> if (This->pulse_stream) {
> -        pulse->unlock();
> +        LeaveCriticalSection(&session_cs);
>          return AUDCLNT_E_ALREADY_INITIALIZED;
>      }
>
> @@ -562,7 +571,7 @@ static HRESULT WINAPI AudioClient_Initialize(IAudioClient3 *iface,
>          if (!(pulse_thread = CreateThread(NULL, 0, pulse_mainloop_thread, event, 0, NULL)))
>          {
>              ERR("Failed to create mainloop thread.\n");
> -            pulse->unlock();
> +            LeaveCriticalSection(&session_cs);
>              CloseHandle(event);
>              return E_FAIL;
>          }
> @@ -577,14 +586,14 @@ static HRESULT WINAPI AudioClient_Initialize(IAudioClient3 *iface,
>      free(name);
>      if (FAILED(hr))
>      {
> -        pulse->unlock();
> +        LeaveCriticalSection(&session_cs);
>          return hr;
>      }
>
> if (!(This->vol = malloc(channel_count * sizeof(*This->vol))))
>      {
>          pulse->release_stream(stream, NULL);
> -        pulse->unlock();
> +        LeaveCriticalSection(&session_cs);
>          return E_OUTOFMEMORY;
>      }
>      for (i = 0; i < channel_count; i++)
> @@ -595,7 +604,7 @@ static HRESULT WINAPI AudioClient_Initialize(IAudioClient3 *iface,
>      {
>          free(This->vol);
>          This->vol = NULL;
> -        pulse->unlock();
> +        LeaveCriticalSection(&session_cs);
>          pulse->release_stream(stream, NULL);
>          return E_OUTOFMEMORY;
>      }
> @@ -605,7 +614,7 @@ static HRESULT WINAPI AudioClient_Initialize(IAudioClient3 *iface,
>      list_add_tail(&This->session->clients, &This->entry);
>      set_stream_volumes(This);
>
> -    pulse->unlock();
> +    LeaveCriticalSection(&session_cs);
>      return S_OK;
>  }
>
> @@ -1445,12 +1454,12 @@ static HRESULT WINAPI AudioStreamVolume_SetAllVolumes(
>      if (count != This->channel_count)
>          return E_INVALIDARG;
>
> -    pulse->lock();
> +    EnterCriticalSection(&session_cs);
>      for (i = 0; i < count; ++i)
>          This->vol[i] = levels[i];
>
> set_stream_volumes(This);
> -    pulse->unlock();
> +    LeaveCriticalSection(&session_cs);
>      return S_OK;
>  }
>
> @@ -1470,10 +1479,10 @@ static HRESULT WINAPI AudioStreamVolume_GetAllVolumes(
>      if (count != This->channel_count)
>          return E_INVALIDARG;
>
> -    pulse->lock();
> +    EnterCriticalSection(&session_cs);
>      for (i = 0; i < count; ++i)
>          levels[i] = This->vol[i];
> -    pulse->unlock();
> +    LeaveCriticalSection(&session_cs);
>      return S_OK;
>  }
>
> @@ -1492,10 +1501,10 @@ static HRESULT WINAPI AudioStreamVolume_SetChannelVolume(
>      if (index >= This->channel_count)
>          return E_INVALIDARG;
>
> -    pulse->lock();
> +    EnterCriticalSection(&session_cs);
>      This->vol[index] = level;
>      set_stream_volumes(This);
> -    pulse->unlock();
> +    LeaveCriticalSection(&session_cs);
>      return S_OK;
>  }
>
> @@ -1514,9 +1523,9 @@ static HRESULT WINAPI AudioStreamVolume_GetChannelVolume(
>      if (index >= This->channel_count)
>          return E_INVALIDARG;
>
> -    pulse->lock();
> +    EnterCriticalSection(&session_cs);
>      *level = This->vol[index];
> -    pulse->unlock();
> +    LeaveCriticalSection(&session_cs);
>      return S_OK;
>  }
>
> @@ -1614,7 +1623,7 @@ static HRESULT WINAPI AudioSessionControl_GetState(IAudioSessionControl2 *iface,
>      if (!state)
>          return NULL_PTR_ERR;
>
> -    pulse->lock();
> +    EnterCriticalSection(&session_cs);
>      if (list_empty(&This->session->clients)) {
>          *state = AudioSessionStateExpired;
>          goto out;
> @@ -1628,7 +1637,7 @@ static HRESULT WINAPI AudioSessionControl_GetState(IAudioSessionControl2 *iface,
>      *state = AudioSessionStateInactive;
>
> out:
> -    pulse->unlock();
> +    LeaveCriticalSection(&session_cs);
>      return S_OK;
>  }
>
> @@ -2004,11 +2013,11 @@ static HRESULT WINAPI SimpleAudioVolume_SetMasterVolume(
>
> TRACE("PulseAudio does not support session volume control\n");
>
> -    pulse->lock();
> +    EnterCriticalSection(&session_cs);
>      session->master_vol = level;
>      LIST_FOR_EACH_ENTRY(client, &This->session->clients, ACImpl, entry)
>          set_stream_volumes(client);
> -    pulse->unlock();
> +    LeaveCriticalSection(&session_cs);
>
> return S_OK;
>  }
> @@ -2041,11 +2050,11 @@ static HRESULT WINAPI SimpleAudioVolume_SetMute(ISimpleAudioVolume *iface,
>      if (context)
>          FIXME("Notifications not supported yet\n");
>
> -    pulse->lock();
> +    EnterCriticalSection(&session_cs);
>      session->mute = mute;
>      LIST_FOR_EACH_ENTRY(client, &This->session->clients, ACImpl, entry)
>          set_stream_volumes(client);
> -    pulse->unlock();
> +    LeaveCriticalSection(&session_cs);
>
> return S_OK;
>  }
> @@ -2148,11 +2157,11 @@ static HRESULT WINAPI ChannelAudioVolume_SetChannelVolume(
>
> TRACE("PulseAudio does not support session volume control\n");
>
> -    pulse->lock();
> +    EnterCriticalSection(&session_cs);
>      session->channel_vols[index] = level;
>      LIST_FOR_EACH_ENTRY(client, &This->session->clients, ACImpl, entry)
>          set_stream_volumes(client);
> -    pulse->unlock();
> +    LeaveCriticalSection(&session_cs);
>
> return S_OK;
>  }
> @@ -2199,12 +2208,12 @@ static HRESULT WINAPI ChannelAudioVolume_SetAllVolumes(
>
> TRACE("PulseAudio does not support session volume control\n");
>
> -    pulse->lock();
> +    EnterCriticalSection(&session_cs);
>      for(i = 0; i < count; ++i)
>          session->channel_vols[i] = levels[i];
>      LIST_FOR_EACH_ENTRY(client, &This->session->clients, ACImpl, entry)
>          set_stream_volumes(client);
> -    pulse->unlock();
> +    LeaveCriticalSection(&session_cs);
>      return S_OK;
>  }
>
> diff --git a/dlls/winepulse.drv/pulse.c b/dlls/winepulse.drv/pulse.c
> index 143406988d4..2511e8fea39 100644
> --- a/dlls/winepulse.drv/pulse.c
> +++ b/dlls/winepulse.drv/pulse.c
> @@ -99,12 +99,12 @@ static pthread_cond_t pulse_cond = PTHREAD_COND_INITIALIZER;
>  UINT8 mult_alaw_sample(UINT8, float);
>  UINT8 mult_ulaw_sample(UINT8, float);
>
> -static void WINAPI pulse_lock(void)
> +static void pulse_lock(void)
>  {
>      pthread_mutex_lock(&pulse_mutex);
>  }
>
> -static void WINAPI pulse_unlock(void)
> +static void pulse_unlock(void)
>  {
>      pthread_mutex_unlock(&pulse_mutex);
>  }
> @@ -178,13 +178,13 @@ static int pulse_poll_func(struct pollfd *ufds, unsigned long nfds, int timeout,
>  static void WINAPI pulse_main_loop(HANDLE event)
>  {
>      int ret;
> +    pulse_lock();
>      pulse_ml = pa_mainloop_new();
>      pa_mainloop_set_poll_func(pulse_ml, pulse_poll_func, NULL);
>      NtSetEvent(event, NULL);
> -    pulse_lock();
>      pa_mainloop_run(pulse_ml, &ret);
> -    pulse_unlock();
>      pa_mainloop_free(pulse_ml);
> +    pulse_unlock();
>  }
>
> static void pulse_contextcallback(pa_context *c, void *userdata)
> @@ -799,11 +799,19 @@ static HRESULT WINAPI pulse_create_stream(const char *name, EDataFlow dataflow,
>      unsigned int i, bufsize_bytes;
>      HRESULT hr;
>
> +    pulse_lock();
> +
>      if (FAILED(hr = pulse_connect(name)))
> +    {
> +        pulse_unlock();
>          return hr;
> +    }
>
> if (!(stream = RtlAllocateHeap(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*stream))))
> +    {
> +        pulse_unlock();
>          return E_OUTOFMEMORY;
> +    }
>
> stream->dataflow = dataflow;
>      for (i = 0; i < ARRAY_SIZE(stream->vol); ++i)
> @@ -867,6 +875,9 @@ static HRESULT WINAPI pulse_create_stream(const char *name, EDataFlow dataflow,
>          }
>      }
>
> +    *channel_count = stream->ss.channels;
> +    *ret = stream;
> +
>  exit:
>      if (FAILED(hr)) {
>          free(stream->local_buffer);
> @@ -875,12 +886,10 @@ exit:
>              pa_stream_unref(stream->stream);
>              RtlFreeHeap(GetProcessHeap(), 0, stream);
>          }
> -        return hr;
>      }
>
> -    *channel_count = stream->ss.channels;
> -    *ret = stream;
> -    return S_OK;
> +    pulse_unlock();
> +    return hr;
>  }
>
> static void WINAPI pulse_release_stream(struct pulse_stream *stream, HANDLE timer)
> @@ -1802,13 +1811,15 @@ static HRESULT WINAPI pulse_set_event_handle(struct pulse_stream *stream, HANDLE
>
> static BOOL WINAPI pulse_is_started(struct pulse_stream *stream)
>  {
> -    return pulse_stream_valid(stream) && stream->started;
> +    BOOL ret;
> +    pulse_lock();
> +    ret = pulse_stream_valid(stream) && stream->started;
> +    pulse_unlock();
> +    return ret;
>  }
>
> static const struct unix_funcs unix_funcs =
>  {
> -    pulse_lock,
> -    pulse_unlock,
>      pulse_main_loop,
>      pulse_create_stream,
>      pulse_release_stream,
> diff --git a/dlls/winepulse.drv/unixlib.h b/dlls/winepulse.drv/unixlib.h
> index 797ed3bdaa5..b8a579ebef4 100644
> --- a/dlls/winepulse.drv/unixlib.h
> +++ b/dlls/winepulse.drv/unixlib.h
> @@ -33,8 +33,6 @@ struct pulse_config
>
> struct unix_funcs
>  {
> -    void (WINAPI *lock)(void);
> -    void (WINAPI *unlock)(void);
>      void (WINAPI *main_loop)(HANDLE event);
>      HRESULT (WINAPI *create_stream)(const char *name, EDataFlow dataflow, AUDCLNT_SHAREMODE mode,
>                                      DWORD flags, REFERENCE_TIME duration, REFERENCE_TIME period,