From 93fd9a2e2b8816d8b4260fd1e5d85069692a6eda Mon Sep 17 00:00:00 2001 From: Christian Speckner Date: Sun, 11 Aug 2024 10:57:47 +0200 Subject: [PATCH] Fix use-after-free on the audio thread when Console is destroyed. --- src/common/SoundNull.hxx | 2 +- src/common/SoundSDL2.cxx | 2 +- src/common/SoundSDL2.hxx | 4 ++-- src/emucore/Console.cxx | 13 +++++++------ src/emucore/Console.hxx | 7 ++++--- src/emucore/Sound.hxx | 2 +- src/os/libretro/SoundLIBRETRO.hxx | 6 +----- 7 files changed, 17 insertions(+), 19 deletions(-) diff --git a/src/common/SoundNull.hxx b/src/common/SoundNull.hxx index 375ab00e0..59b22e496 100644 --- a/src/common/SoundNull.hxx +++ b/src/common/SoundNull.hxx @@ -57,7 +57,7 @@ class SoundNull : public Sound Initializes the sound device. This must be called before any calls are made to derived methods. */ - void open(shared_ptr, EmulationTiming*) override { } + void open(shared_ptr, shared_ptr) override { } /** Sets the sound mute state; sound processing continues. When turned diff --git a/src/common/SoundSDL2.cxx b/src/common/SoundSDL2.cxx index 91b215a6c..debe6143e 100644 --- a/src/common/SoundSDL2.cxx +++ b/src/common/SoundSDL2.cxx @@ -144,7 +144,7 @@ void SoundSDL2::setEnabled(bool enable) // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - void SoundSDL2::open(shared_ptr audioQueue, - EmulationTiming* emulationTiming) + shared_ptr emulationTiming) { const string pre_about = myAboutString; diff --git a/src/common/SoundSDL2.hxx b/src/common/SoundSDL2.hxx index 2a118503c..1dbe582e5 100644 --- a/src/common/SoundSDL2.hxx +++ b/src/common/SoundSDL2.hxx @@ -59,7 +59,7 @@ class SoundSDL2 : public Sound calls are made to derived methods. */ void open(shared_ptr audioQueue, - EmulationTiming* emulationTiming) override; + shared_ptr emulationTiming) override; /** Sets the sound mute state; sound processing continues. When enabled, @@ -164,7 +164,7 @@ class SoundSDL2 : public Sound shared_ptr myAudioQueue; unique_ptr myResampler; - EmulationTiming* myEmulationTiming{nullptr}; + shared_ptr myEmulationTiming; Int16* myCurrentFragment{nullptr}; bool myUnderrun{false}; diff --git a/src/emucore/Console.cxx b/src/emucore/Console.cxx index b8d8fc9da..e1e8d5bef 100644 --- a/src/emucore/Console.cxx +++ b/src/emucore/Console.cxx @@ -120,6 +120,7 @@ Console::Console(OSystem& osystem, unique_ptr& cart, myCart{std::move(cart)}, myAudioSettings{audioSettings} { + myEmulationTiming = make_shared(); myCart->setProperties(&myProperties); // Create subsystems for the console @@ -752,7 +753,7 @@ FBInitStatus Console::initializeVideo(bool full) // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - void Console::initializeAudio() { - myEmulationTiming + (*myEmulationTiming) .updatePlaybackRate(myAudioSettings.sampleRate()) .updatePlaybackPeriod(myAudioSettings.fragmentSize()) .updateAudioQueueExtraFragments(myAudioSettings.bufferSize()) @@ -765,7 +766,7 @@ void Console::initializeAudio() myTIA->setAudioQueue(myAudioQueue); myTIA->setAudioRewindMode(myOSystem.state().mode() != StateManager::Mode::Off); - myOSystem.sound().open(myAudioQueue, &myEmulationTiming); + myOSystem.sound().open(myAudioQueue, myEmulationTiming); } /* Original frying research and code by Fred Quimby. @@ -879,8 +880,8 @@ void Console::setTIAProperties() myTIA->setAdjustVSize(myOSystem.settings().getInt("tia.vsizeadjust")); myTIA->setVcenter(vcenter); - myEmulationTiming.updateFrameLayout(myTIA->frameLayout()); - myEmulationTiming.updateConsoleTiming(myConsoleTiming); + myEmulationTiming->updateFrameLayout(myTIA->frameLayout()); + myEmulationTiming->updateConsoleTiming(myConsoleTiming); } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - @@ -890,8 +891,8 @@ void Console::createAudioQueue() || myProperties.get(PropType::Cart_Sound) == "STEREO"; myAudioQueue = make_shared( - myEmulationTiming.audioFragmentSize(), - myEmulationTiming.audioQueueCapacity(), + myEmulationTiming->audioFragmentSize(), + myEmulationTiming->audioQueueCapacity(), useStereo ); } diff --git a/src/emucore/Console.hxx b/src/emucore/Console.hxx index 49b38d33a..b8e3186c1 100644 --- a/src/emucore/Console.hxx +++ b/src/emucore/Console.hxx @@ -185,7 +185,7 @@ class Console : public Serializable, public ConsoleIO /** Retrieve emulation timing provider. */ - EmulationTiming& emulationTiming() { return myEmulationTiming; } + EmulationTiming& emulationTiming() { return *myEmulationTiming; } /** Toggle left and right controller ports swapping @@ -497,8 +497,9 @@ class Console : public Serializable, public ConsoleIO ConsoleTiming myConsoleTiming{ConsoleTiming::ntsc}; // Emulation timing provider. This ties together the timing of the core emulation loop - // and the parameters that govern audio synthesis - EmulationTiming myEmulationTiming; + // and the parameters that govern audio synthesis. It is used on the audio thread, + // so we make it a shared pointer. + shared_ptr myEmulationTiming; // The audio settings AudioSettings& myAudioSettings; diff --git a/src/emucore/Sound.hxx b/src/emucore/Sound.hxx index e7aa8ed23..c8d9f4941 100644 --- a/src/emucore/Sound.hxx +++ b/src/emucore/Sound.hxx @@ -53,7 +53,7 @@ class Sound Start the sound system, initializing it if necessary. This must be called before any calls are made to derived methods. */ - virtual void open(shared_ptr, EmulationTiming*) = 0; + virtual void open(shared_ptr, shared_ptr) = 0; /** Sets the sound mute state; sound processing continues. When turned diff --git a/src/os/libretro/SoundLIBRETRO.hxx b/src/os/libretro/SoundLIBRETRO.hxx index 8dcf84e0b..c7183bee5 100644 --- a/src/os/libretro/SoundLIBRETRO.hxx +++ b/src/os/libretro/SoundLIBRETRO.hxx @@ -63,10 +63,8 @@ class SoundLIBRETRO : public Sound calls are made to derived methods. */ void open(shared_ptr audioQueue, - EmulationTiming* emulationTiming) override + shared_ptr) override { - myEmulationTiming = emulationTiming; - Logger::debug("SoundLIBRETRO::open started ..."); audioQueue->ignoreOverflows(!myAudioSettings.enabled()); @@ -142,8 +140,6 @@ class SoundLIBRETRO : public Sound shared_ptr myAudioQueue; - EmulationTiming* myEmulationTiming{nullptr}; - Int16* myCurrentFragment{nullptr}; bool myUnderrun{false};