From 13103551cf1085b4b52b340b49a1b33e85526e69 Mon Sep 17 00:00:00 2001 From: Souryo Date: Tue, 14 Nov 2017 00:00:00 -0500 Subject: [PATCH] Netplay: Fixed some potential multithreading issues --- Core/EmulationSettings.cpp | 23 +------------ Core/EmulationSettings.h | 32 ++++++++++++------ Core/GameClient.cpp | 62 +++++++++++++++++------------------ Core/GameClient.h | 7 ++-- Core/GameClientConnection.cpp | 19 ++++++++--- Core/GameClientConnection.h | 2 ++ Core/GameConnection.cpp | 7 ++-- Core/MessageManager.cpp | 5 +++ Core/MessageManager.h | 1 + Core/RewindManager.cpp | 38 +++++++++++---------- Core/RewindManager.h | 6 ++-- Core/SoundMixer.cpp | 5 +-- GUI.NET/Forms/frmMain.cs | 4 ++- 13 files changed, 113 insertions(+), 98 deletions(-) diff --git a/Core/EmulationSettings.cpp b/Core/EmulationSettings.cpp index 077077b2..460fe22d 100644 --- a/Core/EmulationSettings.cpp +++ b/Core/EmulationSettings.cpp @@ -31,6 +31,7 @@ double EmulationSettings::_stereoAngle = 0; double EmulationSettings::_reverbStrength = 0; double EmulationSettings::_reverbDelay = 0; uint32_t EmulationSettings::_crossFeedRatio = 0; +SimpleLock EmulationSettings::_equalizerLock; NesModel EmulationSettings::_model = NesModel::Auto; PpuModel EmulationSettings::_ppuModel = PpuModel::Ppu2C02; @@ -115,28 +116,6 @@ uint8_t EmulationSettings::_paletteLut[11][64] = { /* 2C05-05 */ { 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,15,46,47,48,49,50,51,52,53,54,55,56,57,58,59,60,15,62,63 }, }; -void EmulationSettings::SetEqualizerBands(double *bands, uint32_t bandCount) -{ - Console::Pause(); - _bands.clear(); - _bandGains.clear(); - for(uint32_t i = 0; i < bandCount; i++) { - _bands.push_back(bands[i]); - _bandGains.push_back(0); - } - Console::Resume(); -} - -void EmulationSettings::SetRewindBufferSize(uint32_t seconds) -{ - if(seconds == 0 || _rewindBufferSize == 0) { - Console::Pause(); - RewindManager::ClearBuffer(); - Console::Resume(); - } - _rewindBufferSize = seconds; -} - uint32_t EmulationSettings::GetEmulationSpeed(bool ignoreTurbo) { if(ignoreTurbo) { diff --git a/Core/EmulationSettings.h b/Core/EmulationSettings.h index 09c7ad61..c459f752 100644 --- a/Core/EmulationSettings.h +++ b/Core/EmulationSettings.h @@ -537,6 +537,7 @@ private: static RamPowerOnState _ramPowerOnState; static SimpleLock _shortcutLock; + static SimpleLock _equalizerLock; static SimpleLock _lock; public: @@ -658,29 +659,37 @@ public: _audioSettingsChanged = true; } - static double GetBandGain(int band) + static vector GetBandGains() { - if(band < (int)_bandGains.size()) { - return _bandGains[band]; - } else { - return 0; - } + auto lock = _equalizerLock.AcquireSafe(); + return _bandGains; } static void SetBandGain(int band, double gain) { + auto lock = _equalizerLock.AcquireSafe(); if(band < (int)_bandGains.size()) { _bandGains[band] = gain; + _audioSettingsChanged = true; } - _audioSettingsChanged = true; } - + static vector GetEqualizerBands() { + auto lock = _equalizerLock.AcquireSafe(); return _bands; } - static void SetEqualizerBands(double *bands, uint32_t bandCount); + static void SetEqualizerBands(double *bands, uint32_t bandCount) + { + auto lock = _equalizerLock.AcquireSafe(); + _bands.clear(); + _bandGains.clear(); + for(uint32_t i = 0; i < bandCount; i++) { + _bands.push_back(bands[i]); + _bandGains.push_back(0); + } + } static EqualizerFilterType GetEqualizerFilterType() { @@ -830,7 +839,10 @@ public: return _rewindSpeed; } - static void SetRewindBufferSize(uint32_t seconds); + static void SetRewindBufferSize(uint32_t seconds) + { + _rewindBufferSize = seconds; + } static uint32_t GetRewindBufferSize() { diff --git a/Core/GameClient.cpp b/Core/GameClient.cpp index 31a476bb..1c6e4080 100644 --- a/Core/GameClient.cpp +++ b/Core/GameClient.cpp @@ -9,7 +9,7 @@ using std::thread; #include "ClientConnectionData.h" #include "GameClientConnection.h" -unique_ptr GameClient::Instance; +shared_ptr GameClient::_instance; GameClient::GameClient() { @@ -29,36 +29,42 @@ GameClient::~GameClient() bool GameClient::Connected() { - if(Instance) { - return Instance->_connected; - } else { - return false; - } + shared_ptr instance = _instance; + return instance ? instance->_connected : false; } void GameClient::Connect(shared_ptr connectionData) { - Instance.reset(new GameClient()); - Instance->PrivateConnect(connectionData); - Instance->_clientThread.reset(new thread(&GameClient::Exec, Instance.get())); + _instance.reset(new GameClient()); + + shared_ptr instance = _instance; + if(instance) { + instance->PrivateConnect(connectionData); + instance->_clientThread.reset(new thread(&GameClient::Exec, instance.get())); + } } void GameClient::Disconnect() { - Instance.reset(); + _instance.reset(); +} + +shared_ptr GameClient::GetConnection() +{ + shared_ptr instance = _instance; + return instance ? instance->_connection : nullptr; } void GameClient::PrivateConnect(shared_ptr connectionData) { _stop = false; - _socket.reset(new Socket()); - if(_socket->Connect(connectionData->Host.c_str(), connectionData->Port)) { - _connection.reset(new GameClientConnection(_socket, connectionData)); + shared_ptr socket(new Socket()); + if(socket->Connect(connectionData->Host.c_str(), connectionData->Port)) { + _connection.reset(new GameClientConnection(socket, connectionData)); _connected = true; } else { MessageManager::DisplayMessage("NetPlay", "CouldNotConnect"); _connected = false; - _socket.reset(); } } @@ -71,7 +77,7 @@ void GameClient::Exec() _connection->SendInput(); } else { _connected = false; - _socket.reset(); + _connection->Shutdown(); _connection.reset(); break; } @@ -89,34 +95,26 @@ void GameClient::ProcessNotification(ConsoleNotificationType type, void* paramet uint8_t GameClient::GetControllerState(uint8_t port) { - if(Instance && Instance->_connection) { - return Instance->_connection->GetControllerState(port); - } - - return 0; + shared_ptr connection = GetConnection(); + return connection ? connection->GetControllerState(port) : 0; } void GameClient::SelectController(uint8_t port) { - if(Instance && Instance->_connection) { - Instance->_connection->SelectController(port); + shared_ptr connection = GetConnection(); + if(connection) { + connection->SelectController(port); } } uint8_t GameClient::GetAvailableControllers() { - if(Instance && Instance->_connection) { - return Instance->_connection->GetAvailableControllers(); - } - - return 0; + shared_ptr connection = GetConnection(); + return connection ? connection->GetAvailableControllers() : 0; } uint8_t GameClient::GetControllerPort() { - if(Instance && Instance->_connection) { - return Instance->_connection->GetControllerPort(); - } - - return GameConnection::SpectatorPort; + shared_ptr connection = GetConnection(); + return connection ? connection->GetControllerPort() : GameConnection::SpectatorPort; } \ No newline at end of file diff --git a/Core/GameClient.h b/Core/GameClient.h index b3481488..9cc6c9cf 100644 --- a/Core/GameClient.h +++ b/Core/GameClient.h @@ -10,14 +10,15 @@ class ClientConnectionData; class GameClient : public INotificationListener { private: - static unique_ptr Instance; + static shared_ptr _instance; unique_ptr _clientThread; atomic _stop; - shared_ptr _socket; - unique_ptr _connection; + shared_ptr _connection; bool _connected = false; + static shared_ptr GetConnection(); + void PrivateConnect(shared_ptr connectionData); void Exec(); diff --git a/Core/GameClientConnection.cpp b/Core/GameClientConnection.cpp index 2f5c7cdf..7b6cb7db 100644 --- a/Core/GameClientConnection.cpp +++ b/Core/GameClientConnection.cpp @@ -27,12 +27,20 @@ GameClientConnection::GameClientConnection(shared_ptr socket, shared_ptr GameClientConnection::~GameClientConnection() { - _shutdown = true; - DisableControllers(); + Shutdown(); +} - MessageManager::SendNotification(ConsoleNotificationType::DisconnectedFromServer); - MessageManager::DisplayMessage("NetPlay", "ConnectionLost"); - MessageManager::UnregisterNotificationListener(this); +void GameClientConnection::Shutdown() +{ + if(!_shutdown) { + _shutdown = true; + DisableControllers(); + + EmulationSettings::ClearFlags(EmulationFlags::ForceMaxSpeed); + MessageManager::UnregisterNotificationListener(this); + MessageManager::SendNotification(ConsoleNotificationType::DisconnectedFromServer); + MessageManager::DisplayMessage("NetPlay", "ConnectionLost"); + } } void GameClientConnection::SendHandshake() @@ -49,6 +57,7 @@ void GameClientConnection::SendControllerSelection(uint8_t port) void GameClientConnection::ClearInputData() { + LockHandler lock = _writeLock.AcquireSafe(); for(int i = 0; i < 4; i++) { _inputSize[i] = 0; _inputData[i].clear(); diff --git a/Core/GameClientConnection.h b/Core/GameClientConnection.h index a3dd7564..c591c900 100644 --- a/Core/GameClientConnection.h +++ b/Core/GameClientConnection.h @@ -41,6 +41,8 @@ public: GameClientConnection(shared_ptr socket, shared_ptr connectionData); ~GameClientConnection(); + void Shutdown(); + void ProcessNotification(ConsoleNotificationType type, void* parameter) override; uint8_t GetControllerState(uint8_t port); diff --git a/Core/GameConnection.cpp b/Core/GameConnection.cpp index 273510a1..efa9d152 100644 --- a/Core/GameConnection.cpp +++ b/Core/GameConnection.cpp @@ -20,6 +20,7 @@ GameConnection::GameConnection(shared_ptr socket, shared_ptrRecv((char*)_readBuffer + _readPosition, 0x40000 - _readPosition, 0); if(bytesReceived > 0) { _readPosition += bytesReceived; @@ -32,7 +33,7 @@ bool GameConnection::ExtractMessage(void *buffer, uint32_t &messageLength) if(messageLength > 1000000) { MessageManager::Log("[Netplay] Invalid data received, closing connection."); - _socket->Close(); + Disconnect(); return false; } @@ -71,13 +72,13 @@ NetMessage* GameConnection::ReadMessage() void GameConnection::SendNetMessage(NetMessage &message) { - _socketLock.Acquire(); + auto lock = _socketLock.AcquireSafe(); message.Send(*_socket.get()); - _socketLock.Release(); } void GameConnection::Disconnect() { + auto lock = _socketLock.AcquireSafe(); _socket->Close(); } diff --git a/Core/MessageManager.cpp b/Core/MessageManager.cpp index d7c1136a..cc3924ad 100644 --- a/Core/MessageManager.cpp +++ b/Core/MessageManager.cpp @@ -567,6 +567,7 @@ std::unordered_map MessageManager::_caResources = { std::list MessageManager::_log; SimpleLock MessageManager::_logLock; +SimpleLock MessageManager::_notificationLock; IMessageManager* MessageManager::_messageManager = nullptr; vector MessageManager::_notificationListeners; @@ -651,16 +652,20 @@ string MessageManager::GetLog() void MessageManager::RegisterNotificationListener(INotificationListener* notificationListener) { + auto lock = _notificationLock.AcquireSafe(); MessageManager::_notificationListeners.push_back(notificationListener); } void MessageManager::UnregisterNotificationListener(INotificationListener* notificationListener) { + auto lock = _notificationLock.AcquireSafe(); MessageManager::_notificationListeners.erase(std::remove(MessageManager::_notificationListeners.begin(), MessageManager::_notificationListeners.end(), notificationListener), MessageManager::_notificationListeners.end()); } void MessageManager::SendNotification(ConsoleNotificationType type, void* parameter) { + auto lock = _notificationLock.AcquireSafe(); + //Iterate on a copy to prevent issues if a notification causes a listener to unregister itself vector listeners = MessageManager::_notificationListeners; vector processedListeners; diff --git a/Core/MessageManager.h b/Core/MessageManager.h index 3350e24f..08acd930 100644 --- a/Core/MessageManager.h +++ b/Core/MessageManager.h @@ -22,6 +22,7 @@ private: static std::unordered_map _caResources; static SimpleLock _logLock; + static SimpleLock _notificationLock; static std::list _log; public: diff --git a/Core/RewindManager.cpp b/Core/RewindManager.cpp index e39c7ef7..4b90e718 100644 --- a/Core/RewindManager.cpp +++ b/Core/RewindManager.cpp @@ -28,16 +28,16 @@ RewindManager::~RewindManager() void RewindManager::ClearBuffer() { if(_instance) { - _instance->_history.clear(); - _instance->_historyBackup.clear(); - _instance->_currentHistory = RewindData(); - _instance->_framesToFastForward = 0; - _instance->_videoHistory.clear(); - _instance->_videoHistoryBuilder.clear(); - _instance->_audioHistory.clear(); - _instance->_audioHistoryBuilder.clear(); - _instance->_rewindState = RewindState::Stopped; - _instance->AddHistoryBlock(); + _history.clear(); + _historyBackup.clear(); + _currentHistory = RewindData(); + _framesToFastForward = 0; + _videoHistory.clear(); + _videoHistoryBuilder.clear(); + _audioHistory.clear(); + _audioHistoryBuilder.clear(); + _rewindState = RewindState::Stopped; + _currentHistory = RewindData(); } } @@ -73,6 +73,8 @@ void RewindManager::ProcessNotification(ConsoleNotificationType type, void * par _currentHistory.FrameCount++; break; } + } else { + ClearBuffer(); } } } @@ -80,15 +82,17 @@ void RewindManager::ProcessNotification(ConsoleNotificationType type, void * par void RewindManager::AddHistoryBlock() { uint32_t maxHistorySize = EmulationSettings::GetRewindBufferSize() * 120; - while(_history.size() > maxHistorySize) { - _history.pop_front(); - } + if(maxHistorySize > 0) { + while(_history.size() > maxHistorySize) { + _history.pop_front(); + } - if(_currentHistory.FrameCount > 0) { - _history.push_back(_currentHistory); + if(_currentHistory.FrameCount > 0) { + _history.push_back(_currentHistory); + } + _currentHistory = RewindData(); + _currentHistory.SaveState(); } - _currentHistory = RewindData(); - _currentHistory.SaveState(); } void RewindManager::PopHistory() diff --git a/Core/RewindManager.h b/Core/RewindManager.h index bef781d4..b6a54152 100644 --- a/Core/RewindManager.h +++ b/Core/RewindManager.h @@ -39,6 +39,8 @@ private: void ProcessFrame(void *frameBuffer, uint32_t width, uint32_t height); bool ProcessAudio(int16_t *soundBuffer, uint32_t sampleCount, uint32_t sampleRate); + + void ClearBuffer(); public: RewindManager(); @@ -46,9 +48,7 @@ public: void ProcessNotification(ConsoleNotificationType type, void* parameter) override; void ProcessEndOfFrame(); - - static void ClearBuffer(); - + static void RecordInput(uint8_t port, uint8_t input); static uint8_t GetInput(uint8_t port); diff --git a/Core/SoundMixer.cpp b/Core/SoundMixer.cpp index 9db2a944..87acc400 100644 --- a/Core/SoundMixer.cpp +++ b/Core/SoundMixer.cpp @@ -311,6 +311,7 @@ void SoundMixer::UpdateEqualizers(bool forceUpdate) EqualizerFilterType type = EmulationSettings::GetEqualizerFilterType(); if(type != EqualizerFilterType::None) { vector bands = EmulationSettings::GetEqualizerBands(); + vector bandGains = EmulationSettings::GetBandGains(); if(bands.size() != _eqFrequencyGrid->get_number_of_bands()) { _equalizerLeft.reset(); @@ -332,8 +333,8 @@ void SoundMixer::UpdateEqualizers(bool forceUpdate) } for(unsigned int i = 0; i < _eqFrequencyGrid->get_number_of_bands(); i++) { - _equalizerLeft->change_band_gain_db(i, EmulationSettings::GetBandGain(i)); - _equalizerRight->change_band_gain_db(i, EmulationSettings::GetBandGain(i)); + _equalizerLeft->change_band_gain_db(i, bandGains[i]); + _equalizerRight->change_band_gain_db(i, bandGains[i]); } } else { _equalizerLeft.reset(); diff --git a/GUI.NET/Forms/frmMain.cs b/GUI.NET/Forms/frmMain.cs index 87068347..7a7eeb57 100644 --- a/GUI.NET/Forms/frmMain.cs +++ b/GUI.NET/Forms/frmMain.cs @@ -458,7 +458,9 @@ namespace Mesen.GUI.Forms break; case InteropEmu.ConsoleNotificationType.DisconnectedFromServer: - ConfigManager.Config.ApplyConfig(); + this.BeginInvoke((MethodInvoker)(() => { + ConfigManager.Config.ApplyConfig(); + })); break; case InteropEmu.ConsoleNotificationType.GameStopped: