From 23889bd7c8027f8070480d06b455853da68719ae Mon Sep 17 00:00:00 2001 From: Gareth Francis Date: Fri, 11 May 2018 02:04:44 +0100 Subject: [PATCH] Avoid crash in ~LightsDriver_SystemMessage (#1663) * Avoid crash in ~LightsDriver_SystemMessage This will avoid a crash caused by the fact LIGHTSMAN is destroyed after the other MAN objects * Rework code path for turning lights off on exit Calls to LightsDriver::reset removed from each driver to avoid any crashes LightsDriver::reset renamed to Rest to match surrounding style Added LightsManager::TurnOffAllLights, called before XXXMAN objects are deleted --- src/LightsManager.cpp | 6 ++++++ src/LightsManager.h | 1 + src/StepMania.cpp | 8 ++++++++ src/arch/Lights/LightsDriver.cpp | 2 +- src/arch/Lights/LightsDriver.h | 4 ++-- src/arch/Lights/LightsDriver_Export.cpp | 1 - src/arch/Lights/LightsDriver_LinuxParallel.cpp | 1 - src/arch/Lights/LightsDriver_Linux_PIUIO.cpp | 1 - src/arch/Lights/LightsDriver_SextetStream.cpp | 1 - src/arch/Lights/LightsDriver_SystemMessage.cpp | 6 +++++- src/arch/Lights/LightsDriver_Win32Parallel.cpp | 1 - 11 files changed, 23 insertions(+), 9 deletions(-) diff --git a/src/LightsManager.cpp b/src/LightsManager.cpp index 8845df3996..517ab2f117 100644 --- a/src/LightsManager.cpp +++ b/src/LightsManager.cpp @@ -512,6 +512,12 @@ bool LightsManager::IsEnabled() const return m_vpDrivers.size() >= 1 || PREFSMAN->m_bDebugLights; } +void LightsManager::TurnOffAllLights() +{ + FOREACH( LightsDriver*, m_vpDrivers, iter ) + (*iter)->Reset(); +} + /* * (c) 2003-2004 Chris Danford * All rights reserved. diff --git a/src/LightsManager.h b/src/LightsManager.h index 19a5889fa6..0e8a677291 100644 --- a/src/LightsManager.h +++ b/src/LightsManager.h @@ -67,6 +67,7 @@ public: void BlinkCabinetLight( CabinetLight cl ); void BlinkGameButton( GameInput gi ); void BlinkActorLight( CabinetLight cl ); + void TurnOffAllLights(); void PulseCoinCounter() { ++m_iQueuedCoinCounterPulses; } float GetActorLightLatencySeconds() const; diff --git a/src/StepMania.cpp b/src/StepMania.cpp index c92e9356d9..018ab7923c 100644 --- a/src/StepMania.cpp +++ b/src/StepMania.cpp @@ -286,6 +286,14 @@ void ShutdownGame() if( SOUNDMAN ) SOUNDMAN->Shutdown(); + /* Reset all lights to off. + * This is done before ~LightsManager as some drivers use SCREENMAN + * and similar when setting lights. */ + if( LIGHTSMAN ) + { + LIGHTSMAN->TurnOffAllLights(); + } + SAFE_DELETE( SCREENMAN ); SAFE_DELETE( STATSMAN ); SAFE_DELETE( MESSAGEMAN ); diff --git a/src/arch/Lights/LightsDriver.cpp b/src/arch/Lights/LightsDriver.cpp index 6708e3a0dd..09272a7a9c 100644 --- a/src/arch/Lights/LightsDriver.cpp +++ b/src/arch/Lights/LightsDriver.cpp @@ -30,7 +30,7 @@ void LightsDriver::Create( const RString &sDrivers, vector &Add } } -void LightsDriver::reset() +void LightsDriver::Reset() { LightsState state; ZERO( state.m_bCabinetLights ); diff --git a/src/arch/Lights/LightsDriver.h b/src/arch/Lights/LightsDriver.h index 6340ad7259..69989e02d6 100644 --- a/src/arch/Lights/LightsDriver.h +++ b/src/arch/Lights/LightsDriver.h @@ -17,8 +17,8 @@ public: virtual void Set( const LightsState *ls ) = 0; - // Reset all lights to off - void reset(); + // Reset all lights to off + void Reset(); }; #define REGISTER_LIGHTS_DRIVER_CLASS2( name, x ) \ diff --git a/src/arch/Lights/LightsDriver_Export.cpp b/src/arch/Lights/LightsDriver_Export.cpp index 4bc52d244a..05d69c433b 100644 --- a/src/arch/Lights/LightsDriver_Export.cpp +++ b/src/arch/Lights/LightsDriver_Export.cpp @@ -13,7 +13,6 @@ LightsDriver_Export::LightsDriver_Export() LightsDriver_Export::~LightsDriver_Export() { - LightsDriver::reset(); } void LightsDriver_Export::Set( const LightsState *ls ) diff --git a/src/arch/Lights/LightsDriver_LinuxParallel.cpp b/src/arch/Lights/LightsDriver_LinuxParallel.cpp index e0584128c9..ff036e9d2d 100644 --- a/src/arch/Lights/LightsDriver_LinuxParallel.cpp +++ b/src/arch/Lights/LightsDriver_LinuxParallel.cpp @@ -24,7 +24,6 @@ LightsDriver_LinuxParallel::LightsDriver_LinuxParallel() LightsDriver_LinuxParallel::~LightsDriver_LinuxParallel() { - LightsDriver::reset(); // Reset all bits to zero and free the port's permissions outb( 0, PORT_ADDRESS ); ioperm( PORT_ADDRESS, 1, 0 ); diff --git a/src/arch/Lights/LightsDriver_Linux_PIUIO.cpp b/src/arch/Lights/LightsDriver_Linux_PIUIO.cpp index 6c65ccf321..bf2536bb3f 100644 --- a/src/arch/Lights/LightsDriver_Linux_PIUIO.cpp +++ b/src/arch/Lights/LightsDriver_Linux_PIUIO.cpp @@ -32,7 +32,6 @@ LightsDriver_Linux_PIUIO::LightsDriver_Linux_PIUIO() LightsDriver_Linux_PIUIO::~LightsDriver_Linux_PIUIO() { - LightsDriver::reset(); if( fd >= 0 ) close(fd); } diff --git a/src/arch/Lights/LightsDriver_SextetStream.cpp b/src/arch/Lights/LightsDriver_SextetStream.cpp index 40f79cc574..862590d156 100644 --- a/src/arch/Lights/LightsDriver_SextetStream.cpp +++ b/src/arch/Lights/LightsDriver_SextetStream.cpp @@ -210,7 +210,6 @@ LightsDriver_SextetStream::LightsDriver_SextetStream() LightsDriver_SextetStream::~LightsDriver_SextetStream() { - LightsDriver::reset(); if(IMPL != NULL) { delete IMPL; diff --git a/src/arch/Lights/LightsDriver_SystemMessage.cpp b/src/arch/Lights/LightsDriver_SystemMessage.cpp index 9aeae29c6f..edb8b6df27 100644 --- a/src/arch/Lights/LightsDriver_SystemMessage.cpp +++ b/src/arch/Lights/LightsDriver_SystemMessage.cpp @@ -12,11 +12,15 @@ LightsDriver_SystemMessage::LightsDriver_SystemMessage() LightsDriver_SystemMessage::~LightsDriver_SystemMessage() { - LightsDriver::reset(); } void LightsDriver_SystemMessage::Set( const LightsState *ls ) { + if (!PREFSMAN || !LIGHTSMAN || !SCREENMAN) + { + return; + } + if( !PREFSMAN->m_bDebugLights ) return; diff --git a/src/arch/Lights/LightsDriver_Win32Parallel.cpp b/src/arch/Lights/LightsDriver_Win32Parallel.cpp index 3272160672..93e52b15d4 100644 --- a/src/arch/Lights/LightsDriver_Win32Parallel.cpp +++ b/src/arch/Lights/LightsDriver_Win32Parallel.cpp @@ -57,7 +57,6 @@ LightsDriver_Win32Parallel::LightsDriver_Win32Parallel() LightsDriver_Win32Parallel::~LightsDriver_Win32Parallel() { - LightsDriver::reset(); FreeLibrary( hDLL ); }