Second attempt at fixing Windows Start/Stop crashes (#881)

* Use shared setHelperRealTime in internal WASAPI threads.

* Capture this instead of all by reference in internal WASAPI threads.

* Clean up refcounts of COM objects.

* Missed release of default audio device.

* Event handle needs to be closed on stop().

* Fix cross-compile definitions to allow asan to be used in the first place.

* Make sure TX/RX thread stops before audio devices do.

cmake/GetDependencies.cmake.in

@@ -118,6 +118,9 @@ set( _windlls
 
     # Filter Python DLLs as they're already included via install()
     python312.dll
+
+    # Additional DLLs needed by Address Sanitizer
+    api-ms-win-core-synch-l1-2-0.dll
 )
 list(REMOVE_ITEM _deps ${_windlls})
 

cross-compile/freedv-mingw-llvm-aarch64.cmake

@@ -11,9 +11,9 @@ set(CMAKE_AR ${triple}-ar)
 set(CMAKE_RANLIB ${triple}-ranlib)
 set(CMAKE_RC_COMPILER ${triple}-windres)
 
-set(CMAKE_C_FLAGS "-Wno-unused-command-line-argument -gcodeview")
+set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wno-unused-command-line-argument -gcodeview")
-set(CMAKE_CXX_FLAGS "-Wno-unused-command-line-argument -gcodeview")
+set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-unused-command-line-argument -gcodeview")
-set(CMAKE_EXE_LINKER_FLAGS -Wl,--pdb=)
+set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,--pdb=")
 
 # For make package use.
 set(CMAKE_OBJDUMP ${triple}-objdump)

cross-compile/freedv-mingw-llvm-x86_64.cmake

@@ -12,9 +12,9 @@ set(CMAKE_AR ${triple}-ar)
 set(CMAKE_RANLIB ${triple}-ranlib)
 set(CMAKE_RC_COMPILER ${triple}-windres)
 
-set(CMAKE_C_FLAGS "-Wno-unused-command-line-argument -gcodeview")
+set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wno-unused-command-line-argument -gcodeview")
-set(CMAKE_CXX_FLAGS "-Wno-unused-command-line-argument -gcodeview")
+set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-unused-command-line-argument -gcodeview")
-set(CMAKE_EXE_LINKER_FLAGS -Wl,--pdb=)
+set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,--pdb=")
 
 # For make package use.
 set(CMAKE_OBJDUMP ${triple}-objdump)

src/audio/WASAPIAudioDevice.cpp

@@ -34,7 +34,7 @@
 // Nanoseconds per REFERENCE_TIME unit
 #define NS_PER_REFTIME (100)
 
-thread_local HANDLE WASAPIAudioDevice::helperTask_ = nullptr;
+thread_local HANDLE WASAPIAudioDevice::HelperTask_ = nullptr;
     
 WASAPIAudioDevice::WASAPIAudioDevice(IAudioClient* client, IAudioEngine::AudioDirection direction, int sampleRate, int numChannels)
     : client_(client)
@@ -45,13 +45,12 @@ WASAPIAudioDevice::WASAPIAudioDevice(IAudioClient* client, IAudioEngine::AudioDi
     , numChannels_(numChannels)
     , bufferFrameCount_(0)
     , initialized_(false)
-    , lowLatencyTask_(nullptr)
     , latencyFrames_(0)
     , renderCaptureEvent_(nullptr)
     , isRenderCaptureRunning_(false)
     , semaphore_(nullptr)
 {
-    // empty
+    client_->AddRef();
 }
 
 WASAPIAudioDevice::~WASAPIAudioDevice()
@@ -291,7 +290,7 @@ void WASAPIAudioDevice::start()
 
         // Start render/capture thread.
         isRenderCaptureRunning_ = true;
-        renderCaptureThread_ = std::thread([&]() {
+        renderCaptureThread_ = std::thread([this]() {
             log_info("Starting render/capture thread");
 
             HRESULT res = CoInitializeEx(nullptr, COINIT_MULTITHREADED | COINIT_DISABLE_OLE1DDE);
@@ -299,14 +298,21 @@ void WASAPIAudioDevice::start()
             {
                 log_warn("Could not initialize COM (res = %d)", res);
             }
-
+            
-            // Temporarily raise priority of task
+            // Increment refcounts of COM objects used by thread
-            DWORD taskIndex = 0;
+            // to avoid instability during stop/restart.
-            lowLatencyTask_ = AvSetMmThreadCharacteristics(TEXT("Pro Audio"), &taskIndex);
+            client_->AddRef();
-            if (lowLatencyTask_ == nullptr)
+            if (renderClient_ != nullptr)
             {
-                log_warn("Could not increase thread priority");
+                renderClient_->AddRef();
             }
+            if (captureClient_ != nullptr)
+            {
+                captureClient_->AddRef();
+            }
+            
+            // Temporarily raise priority of task
+            setHelperRealTime();
 
             while (isRenderCaptureRunning_)
             {
@@ -326,12 +332,19 @@ void WASAPIAudioDevice::start()
 
             log_info("Exiting render/capture thread");
 
-            if (lowLatencyTask_ != nullptr)
+            clearHelperRealTime();     
+            
+            // Decrement refcounts prior to exit.
+            client_->Release();
+            if (renderClient_ != nullptr)
             {
-                AvRevertMmThreadCharacteristics(lowLatencyTask_);
+                renderClient_->Release();
-                lowLatencyTask_ = nullptr;
             }
-
+            if (captureClient_ != nullptr)
+            {
+                captureClient_->Release();
+            }
+                   
             CoUninitialize();
         });
 
@@ -369,6 +382,12 @@ void WASAPIAudioDevice::stop()
             }
         }
 
+        if (renderCaptureEvent_ != nullptr)
+        {
+            CloseHandle(renderCaptureEvent_);
+            renderCaptureEvent_ = nullptr;
+        }
+        
         if (renderClient_ != nullptr)
         {
             renderClient_->Release();
@@ -412,8 +431,8 @@ int WASAPIAudioDevice::getLatencyInMicroseconds()
 void WASAPIAudioDevice::setHelperRealTime()
 {
     DWORD taskIndex = 0;
-    helperTask_ = AvSetMmThreadCharacteristics(TEXT("Pro Audio"), &taskIndex);
+    HelperTask_ = AvSetMmThreadCharacteristics(TEXT("Pro Audio"), &taskIndex);
-    if (helperTask_ == nullptr)
+    if (HelperTask_ == nullptr)
     {
         log_warn("Could not increase thread priority");
     }
@@ -445,10 +464,10 @@ void WASAPIAudioDevice::stopRealTimeWork()
 
 void WASAPIAudioDevice::clearHelperRealTime()
 {
-    if (helperTask_ != nullptr)
+    if (HelperTask_ != nullptr)
     {
-        AvRevertMmThreadCharacteristics(helperTask_);
+        AvRevertMmThreadCharacteristics(HelperTask_);
-        helperTask_ = nullptr;
+        HelperTask_ = nullptr;
     }
 }
 

src/audio/WASAPIAudioDevice.h

@@ -79,7 +79,6 @@ private:
     int numChannels_;
     UINT32 bufferFrameCount_;
     bool initialized_;
-    HANDLE lowLatencyTask_;
     int latencyFrames_;
     std::thread renderCaptureThread_;
     HANDLE renderCaptureEvent_;
@@ -89,7 +88,7 @@ private:
     void renderAudio_();
     void captureAudio_();
     
-    static thread_local HANDLE helperTask_;
+    static thread_local HANDLE HelperTask_;
 };
 
 #endif // WASAPI_AUDIO_DEVICE_H

src/audio/WASAPIAudioEngine.cpp

@@ -218,6 +218,7 @@ AudioDeviceSpecification WASAPIAudioEngine::getDefaultAudioDevice(AudioDirection
             if (defaultSpec.name == spec.name)
             {
                 prom->set_value(spec);
+                defaultDevice->Release();
                 return;
             }
         }
@@ -303,6 +304,9 @@ std::shared_ptr<IAudioDevice> WASAPIAudioEngine::getAudioDevice(wxString deviceN
 
                 auto devPtr = new WASAPIAudioDevice(client, direction, sampleRate, numChannels);
                 result = std::shared_ptr<IAudioDevice>(devPtr);
+                
+                client->Release();
+                device->Release();
             }
         }
         prom->set_value(result);
@@ -395,7 +399,7 @@ AudioDeviceSpecification WASAPIAudioEngine::getDeviceSpecification_(IMMDevice* d
         return AudioDeviceSpecification::GetInvalidDevice();
     }
 
-    WAVEFORMATEX* streamFormat;
+    WAVEFORMATEX* streamFormat = nullptr;
     hr = audioClient->GetMixFormat(&streamFormat);
     if (FAILED(hr))
     {

src/main.cpp

@@ -2855,6 +2855,9 @@ void MainFrame::stopRxStream()
 
         if (m_txThread)
         {
+            m_txThread->terminateThread();
+            m_txThread->Wait();
+            
             if (txInSoundDevice)
             {
                 txInSoundDevice->stop();
@@ -2867,15 +2870,15 @@ void MainFrame::stopRxStream()
                 txOutSoundDevice.reset();
             }
             
-            m_txThread->terminateThread();
-            m_txThread->Wait();
-            
             delete m_txThread;
             m_txThread = nullptr;
         }
 
         if (m_rxThread)
         {
+            m_rxThread->terminateThread();
+            m_rxThread->Wait();
+            
             if (rxInSoundDevice)
             {
                 rxInSoundDevice->stop();
@@ -2888,9 +2891,6 @@ void MainFrame::stopRxStream()
                 rxOutSoundDevice.reset();
             }
             
-            m_rxThread->terminateThread();
-            m_rxThread->Wait();
-            
             delete m_txThread;
             m_rxThread = nullptr;
         }

src/pipeline/TxRxThread.cpp

@@ -515,7 +515,8 @@ void* TxRxThread::Entry()
 
 void TxRxThread::OnExit() 
 { 
-    // No actions required for exit.
+    // Free allocated buffer.
+    inputSamples_ = nullptr; 
 }
 
 void TxRxThread::terminateThread()

src/pipeline/TxRxThread.h

@@ -58,6 +58,12 @@ public:
             new short[std::max(inputSampleRate_, outputSampleRate_)], 
             std::default_delete<short[]>());
     }
+    
+    virtual ~TxRxThread()
+    {
+        // Free allocated buffer
+        inputSamples_ = nullptr;
+    }
 
     // thread execution starts here
     void *Entry();