From 603fe2534967dc4bf03ea6d9a36d72048a89acdc Mon Sep 17 00:00:00 2001 From: mathieui Date: Mon, 9 Mar 2015 17:31:13 +0100 Subject: [PATCH 1/6] =?UTF-8?q?NetPlay:=20use=20a=20workaround=20from=20co?= =?UTF-8?q?mex=E2=80=99s=20dc-netplay=20to=20interrupt=20enet?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Source/Core/Core/NetPlayClient.cpp | 110 +++++++++++++-------- Source/Core/Core/NetPlayClient.h | 7 +- Source/Core/Core/NetPlayServer.cpp | 149 ++++++++++++++++------------- Source/Core/Core/NetPlayServer.h | 6 +- 4 files changed, 163 insertions(+), 109 deletions(-) diff --git a/Source/Core/Core/NetPlayClient.cpp b/Source/Core/Core/NetPlayClient.cpp index b2012c10ce..2e792233fc 100644 --- a/Source/Core/Core/NetPlayClient.cpp +++ b/Source/Core/Core/NetPlayClient.cpp @@ -416,7 +416,6 @@ unsigned int NetPlayClient::OnData(sf::Packet& packet) spac << (MessageId)NP_MSG_PONG; spac << ping_key; - std::lock_guard lks(m_crit.send); Send(spac); } break; @@ -480,6 +479,33 @@ void NetPlayClient::Disconnect() m_server = nullptr; } +void NetPlayClient::RunOnThread(std::function func) +{ + { + std::lock_guard lkq(m_crit.run_queue_write); + m_run_queue.Push(func); + } + WakeupThread(m_client); +} + +void NetPlayClient::WakeupThread(ENetHost* host) +{ + // Send ourselves a spurious message. This is hackier than it should be. + // comex reported this as https://github.com/lsalzman/enet/issues/23, so + // hopefully there will be a better way to do it in the future. + ENetAddress address; + if (host->address.port != 0) + address.port = host->address.port; + else + enet_socket_get_address(host->socket, &address); + address.host = 0x0100007f; // localhost + u8 byte = 0; + ENetBuffer buf; + buf.data = &byte; + buf.dataLength = 1; + enet_socket_send(host->socket, &address, &buf, 1); +} + // called from ---NETPLAY--- thread void NetPlayClient::ThreadFunc() { @@ -487,11 +513,13 @@ void NetPlayClient::ThreadFunc() { ENetEvent netEvent; int net; + if (m_traversal_client) + m_traversal_client->HandleResends(); + net = enet_host_service(m_client, &netEvent, 4); + while (!m_run_queue.Empty()) { - std::lock_guard lks(m_crit.send); - if (m_traversal_client) - m_traversal_client->HandleResends(); - net = enet_host_service(m_client, &netEvent, 4); + m_run_queue.Front()(); + m_run_queue.Pop(); } if (net > 0) { @@ -517,7 +545,6 @@ void NetPlayClient::ThreadFunc() break; } } - } Disconnect(); @@ -577,57 +604,57 @@ void NetPlayClient::GetPlayers(std::vector &player_list) // called from ---GUI--- thread void NetPlayClient::SendChatMessage(const std::string& msg) { - sf::Packet spac; - spac << (MessageId)NP_MSG_CHAT_MESSAGE; - spac << msg; - - std::lock_guard lks(m_crit.send); - Send(spac); + RunOnThread([msg, this]() { + sf::Packet spac; + spac << (MessageId)NP_MSG_CHAT_MESSAGE; + spac << msg; + Send(spac); + }); } // called from ---CPU--- thread void NetPlayClient::SendPadState(const PadMapping in_game_pad, const GCPadStatus& pad) { - // send to server sf::Packet spac; spac << (MessageId)NP_MSG_PAD_DATA; spac << in_game_pad; spac << pad.button << pad.analogA << pad.analogB << pad.stickX << pad.stickY << pad.substickX << pad.substickY << pad.triggerLeft << pad.triggerRight; - std::lock_guard lks(m_crit.send); - Send(spac); + RunOnThread([spac, this]() mutable { + // send to server + Send(spac); + }); } // called from ---CPU--- thread void NetPlayClient::SendWiimoteState(const PadMapping in_game_pad, const NetWiimote& nw) { - // send to server - sf::Packet spac; - spac << (MessageId)NP_MSG_WIIMOTE_DATA; - spac << in_game_pad; - spac << (u8)nw.size(); - for (auto it : nw) - { - spac << it; - } - - std::lock_guard lks(m_crit.send); - Send(spac); + RunOnThread([=]() { + // send to server + sf::Packet spac; + spac << (MessageId)NP_MSG_WIIMOTE_DATA; + spac << in_game_pad; + spac << (u8)nw.size(); + for (auto it : nw) + { + spac << it; + } + Send(spac); + }); } // called from ---GUI--- thread bool NetPlayClient::StartGame(const std::string &path) { - std::lock_guard lkg(m_crit.game); - - // tell server i started the game - sf::Packet spac; - spac << (MessageId)NP_MSG_START_GAME; - spac << m_current_game; - spac << (char *)&g_NetPlaySettings; - - std::lock_guard lks(m_crit.send); - Send(spac); + RunOnThread([this](){ + std::lock_guard lkg(m_crit.game); + // tell server i started the game + sf::Packet spac; + spac << (MessageId)NP_MSG_START_GAME; + spac << m_current_game; + spac << (char *)&g_NetPlaySettings; + Send(spac); + }); if (m_is_running) { @@ -954,6 +981,7 @@ bool NetPlayClient::StopGame() return true; } +// called from ---GUI--- thread void NetPlayClient::Stop() { if (m_is_running == false) @@ -976,9 +1004,11 @@ void NetPlayClient::Stop() // tell the server to stop if we have a pad mapped in game. if (isPadMapped) { - sf::Packet spac; - spac << (MessageId)NP_MSG_STOP_GAME; - Send(spac); + RunOnThread([this](){ + sf::Packet spac; + spac << (MessageId)NP_MSG_STOP_GAME; + Send(spac); + }); } } diff --git a/Source/Core/Core/NetPlayClient.h b/Source/Core/Core/NetPlayClient.h index ab5763d313..7b2e3e159a 100644 --- a/Source/Core/Core/NetPlayClient.h +++ b/Source/Core/Core/NetPlayClient.h @@ -47,6 +47,8 @@ class NetPlayClient : public TraversalClientClient { public: void ThreadFunc(); + void RunOnThread(std::function func); + void WakeupThread(ENetHost* host); NetPlayClient(const std::string& address, const u16 port, NetPlayUI* dialog, const std::string& name, bool traversal, std::string centralServer, u16 centralPort); ~NetPlayClient(); @@ -92,9 +94,12 @@ protected: { std::recursive_mutex game; // lock order - std::recursive_mutex players, send; + std::recursive_mutex players; + std::recursive_mutex run_queue_write; } m_crit; + Common::FifoQueue, false> m_run_queue; + Common::FifoQueue m_pad_buffer[4]; Common::FifoQueue m_wiimote_buffer[4]; diff --git a/Source/Core/Core/NetPlayServer.cpp b/Source/Core/Core/NetPlayServer.cpp index 1dcd5c1c8f..7b96b14846 100644 --- a/Source/Core/Core/NetPlayServer.cpp +++ b/Source/Core/Core/NetPlayServer.cpp @@ -92,7 +92,6 @@ NetPlayServer::NetPlayServer(const u16 port, bool traversal, std::string central serverAddr.port = port; m_server = enet_host_create(&serverAddr, 10, 3, 0, 0); } - if (m_server != nullptr) { is_connected = true; @@ -117,7 +116,6 @@ void NetPlayServer::ThreadFunc() spac << (MessageId)NP_MSG_PING; spac << m_ping_key; - std::lock_guard lks(m_crit.send); m_ping_timer.Start(); SendToClients(spac); m_update_pings = false; @@ -125,11 +123,13 @@ void NetPlayServer::ThreadFunc() ENetEvent netEvent; int net; + if (m_traversal_client) + m_traversal_client->HandleResends(); + net = enet_host_service(m_server, &netEvent, 1000); + while (!m_run_queue.Empty()) { - std::lock_guard lks(m_crit.send); - if (m_traversal_client) - m_traversal_client->HandleResends(); - net = enet_host_service(m_server, &netEvent, 4); + m_run_queue.Front()(); + m_run_queue.Pop(); } if (net > 0) { @@ -149,7 +149,6 @@ void NetPlayServer::ThreadFunc() sf::Packet spac; spac << (MessageId)error; // don't need to lock, this client isn't in the client map - std::lock_guard lks(m_crit.send); Send(accept_peer, spac); if (netEvent.peer->data) { @@ -272,52 +271,46 @@ unsigned int NetPlayServer::OnConnect(ENetPeer* socket) } } + // send join message to already connected clients + sf::Packet spac; + spac << (MessageId)NP_MSG_PLAYER_JOIN; + spac << player.pid << player.name << player.revision; + SendToClients(spac); + + // send new client success message with their id + spac.clear(); + spac << (MessageId)0; + spac << player.pid; + Send(player.socket, spac); + + // send new client the selected game + if (m_selected_game != "") { - std::lock_guard lks(m_crit.send); + spac.clear(); + spac << (MessageId)NP_MSG_CHANGE_GAME; + spac << m_selected_game; + Send(player.socket, spac); + } - // send join message to already connected clients - sf::Packet spac; + // send the pad buffer value + spac.clear(); + spac << (MessageId)NP_MSG_PAD_BUFFER; + spac << (u32)m_target_buffer_size; + Send(player.socket, spac); + + // sync values with new client + for (const auto& p : m_players) + { + spac.clear(); spac << (MessageId)NP_MSG_PLAYER_JOIN; - spac << player.pid << player.name << player.revision; - SendToClients(spac); - - // send new client success message with their id - spac.clear(); - spac << (MessageId)0; - spac << player.pid; + spac << p.second.pid << p.second.name << p.second.revision; Send(player.socket, spac); - - // send new client the selected game - if (m_selected_game != "") - { - spac.clear(); - spac << (MessageId)NP_MSG_CHANGE_GAME; - spac << m_selected_game; - Send(player.socket, spac); - } - - // send the pad buffer value - spac.clear(); - spac << (MessageId)NP_MSG_PAD_BUFFER; - spac << (u32)m_target_buffer_size; - Send(player.socket, spac); - - // sync values with new client - for (const auto& p : m_players) - { - spac.clear(); - spac << (MessageId)NP_MSG_PLAYER_JOIN; - spac << p.second.pid << p.second.name << p.second.revision; - Send(player.socket, spac); - } - - } // unlock send + } // add client to the player list { std::lock_guard lkp(m_crit.players); m_players.insert(std::pair(*(PlayerId *)player.socket->data, player)); - std::lock_guard lks(m_crit.send); UpdatePadMapping(); // sync pad mappings with everyone UpdateWiimoteMapping(); } @@ -343,7 +336,6 @@ unsigned int NetPlayServer::OnDisconnect(Client& player) sf::Packet spac; spac << (MessageId)NP_MSG_DISABLE_GAME; // this thread doesn't need players lock - std::lock_guard lks(m_crit.send); SendToClients(spac, 1); break; } @@ -362,7 +354,6 @@ unsigned int NetPlayServer::OnDisconnect(Client& player) m_players.erase(it); // alert other players of disconnect - std::lock_guard lks(m_crit.send); SendToClients(spac); for (PadMapping& mapping : m_pad_map) @@ -451,9 +442,37 @@ void NetPlayServer::AdjustPadBufferSize(unsigned int size) spac << (MessageId)NP_MSG_PAD_BUFFER; spac << (u32)m_target_buffer_size; - std::lock_guard lkp(m_crit.players); - std::lock_guard lks(m_crit.send); - SendToClients(spac); + RunOnThread([spac, this]() mutable { + std::lock_guard lkp(m_crit.players); + SendToClients(spac); + }); +} + +void NetPlayServer::RunOnThread(std::function func) +{ + { + std::lock_guard lkq(m_crit.run_queue_write); + m_run_queue.Push(func); + } + WakeupThread(m_server); +} + +void NetPlayServer::WakeupThread(ENetHost* host) +{ + // Send ourselves a spurious message. This is hackier than it should be. + // comex reported this as https://github.com/lsalzman/enet/issues/23, so + // hopefully there will be a better way to do it in the future. + ENetAddress address; + if (host->address.port != 0) + address.port = host->address.port; + else + enet_socket_get_address(host->socket, &address); + address.host = 0x0100007f; // localhost + u8 byte = 0; + ENetBuffer buf; + buf.data = &byte; + buf.dataLength = 1; + enet_socket_send(host->socket, &address, &buf, 1); } // called from ---NETPLAY--- thread @@ -478,10 +497,7 @@ unsigned int NetPlayServer::OnData(sf::Packet& packet, Client& player) spac << player.pid; spac << msg; - { - std::lock_guard lks(m_crit.send); - SendToClients(spac, player.pid); - } + SendToClients(spac, player.pid); } break; @@ -505,7 +521,6 @@ unsigned int NetPlayServer::OnData(sf::Packet& packet, Client& player) spac << (MessageId)NP_MSG_PAD_DATA; spac << map << pad.button << pad.analogA << pad.analogB << pad.stickX << pad.stickY << pad.substickX << pad.substickY << pad.triggerLeft << pad.triggerRight; - std::lock_guard lks(m_crit.send); SendToClients(spac, player.pid); } break; @@ -538,7 +553,6 @@ unsigned int NetPlayServer::OnData(sf::Packet& packet, Client& player) for (const u8& byte : data) spac << byte; - std::lock_guard lks(m_crit.send); SendToClients(spac, player.pid); } break; @@ -559,7 +573,6 @@ unsigned int NetPlayServer::OnData(sf::Packet& packet, Client& player) spac << player.pid; spac << player.ping; - std::lock_guard lks(m_crit.send); SendToClients(spac); } break; @@ -577,7 +590,6 @@ unsigned int NetPlayServer::OnData(sf::Packet& packet, Client& player) spac << (MessageId)NP_MSG_STOP_GAME; std::lock_guard lkp(m_crit.players); - std::lock_guard lks(m_crit.send); SendToClients(spac); m_is_running = false; @@ -601,7 +613,7 @@ void NetPlayServer::OnTraversalStateChanged() m_dialog->Update(); } -// called from ---GUI--- thread / and ---NETPLAY--- thread +// called from ---GUI--- thread void NetPlayServer::SendChatMessage(const std::string& msg) { sf::Packet spac; @@ -609,9 +621,10 @@ void NetPlayServer::SendChatMessage(const std::string& msg) spac << (PlayerId)0; // server id always 0 spac << msg; - std::lock_guard lkp(m_crit.players); - std::lock_guard lks(m_crit.send); - SendToClients(spac); + RunOnThread([spac, this]() mutable { + std::lock_guard lkp(m_crit.players); + SendToClients(spac); + }); } // called from ---GUI--- thread @@ -626,9 +639,10 @@ bool NetPlayServer::ChangeGame(const std::string &game) spac << (MessageId)NP_MSG_CHANGE_GAME; spac << game; - std::lock_guard lkp(m_crit.players); - std::lock_guard lks(m_crit.send); - SendToClients(spac); + RunOnThread([spac, this]() mutable { + std::lock_guard lkp(m_crit.players); + SendToClients(spac); + }); return true; } @@ -666,9 +680,10 @@ bool NetPlayServer::StartGame() spac << (u32)g_netplay_initial_gctime; spac << (u32)g_netplay_initial_gctime << 32; - std::lock_guard lkp(m_crit.players); - std::lock_guard lks(m_crit.send); - SendToClients(spac); + RunOnThread([spac, this]() mutable { + std::lock_guard lkp(m_crit.players); + SendToClients(spac); + }); m_is_running = true; diff --git a/Source/Core/Core/NetPlayServer.h b/Source/Core/Core/NetPlayServer.h index 5e8ac76411..21f7ea9ede 100644 --- a/Source/Core/Core/NetPlayServer.h +++ b/Source/Core/Core/NetPlayServer.h @@ -20,6 +20,8 @@ class NetPlayServer : public TraversalClientClient { public: void ThreadFunc(); + void RunOnThread(std::function func); + void WakeupThread(ENetHost* host); NetPlayServer(const u16 port, bool traversal, std::string centralServer, u16 centralPort); ~NetPlayServer(); @@ -101,11 +103,13 @@ private: { std::recursive_mutex game; // lock order - std::recursive_mutex players, send; + std::recursive_mutex players; + std::recursive_mutex run_queue_write; } m_crit; std::string m_selected_game; std::thread m_thread; + Common::FifoQueue, false> m_run_queue; ENetHost* m_server; TraversalClient* m_traversal_client; From 44d7207a1c288f216c4d1fa517b8fb0aa94bc001 Mon Sep 17 00:00:00 2001 From: mathieui Date: Mon, 9 Mar 2015 18:37:02 +0100 Subject: [PATCH 2/6] NetPlay: add a Common/ENetUtil namespace Move WakeupThread in it --- Source/Core/Common/CMakeLists.txt | 1 + Source/Core/Common/Common.vcxproj | 4 +++- Source/Core/Common/Common.vcxproj.filters | 2 ++ Source/Core/Common/ENetUtil.cpp | 28 +++++++++++++++++++++++ Source/Core/Common/ENetUtil.h | 15 ++++++++++++ Source/Core/Core/NetPlayClient.cpp | 20 +--------------- Source/Core/Core/NetPlayClient.h | 2 +- Source/Core/Core/NetPlayServer.cpp | 20 +--------------- Source/Core/Core/NetPlayServer.h | 2 +- 9 files changed, 53 insertions(+), 41 deletions(-) create mode 100644 Source/Core/Common/ENetUtil.cpp create mode 100644 Source/Core/Common/ENetUtil.h diff --git a/Source/Core/Common/CMakeLists.txt b/Source/Core/Common/CMakeLists.txt index e80b2cb786..6c3b6ae79b 100644 --- a/Source/Core/Common/CMakeLists.txt +++ b/Source/Core/Common/CMakeLists.txt @@ -1,6 +1,7 @@ set(SRCS BreakPoints.cpp CDUtils.cpp ColorUtil.cpp + ENetUtil.cpp FileSearch.cpp FileUtil.cpp GekkoDisassembler.cpp diff --git a/Source/Core/Common/Common.vcxproj b/Source/Core/Common/Common.vcxproj index ced0c431df..7b2c278cd1 100644 --- a/Source/Core/Common/Common.vcxproj +++ b/Source/Core/Common/Common.vcxproj @@ -51,6 +51,7 @@ + @@ -94,6 +95,7 @@ + @@ -146,4 +148,4 @@ - \ No newline at end of file + diff --git a/Source/Core/Common/Common.vcxproj.filters b/Source/Core/Common/Common.vcxproj.filters index 8545274fc4..ffaf7d6be8 100644 --- a/Source/Core/Common/Common.vcxproj.filters +++ b/Source/Core/Common/Common.vcxproj.filters @@ -25,6 +25,7 @@ + @@ -78,6 +79,7 @@ + diff --git a/Source/Core/Common/ENetUtil.cpp b/Source/Core/Common/ENetUtil.cpp new file mode 100644 index 0000000000..7269893f90 --- /dev/null +++ b/Source/Core/Common/ENetUtil.cpp @@ -0,0 +1,28 @@ +// Copyright 2015 Dolphin Emulator Project +// Licensed under GPLv2 +// Refer to the license.txt file included. + +#include "ENetUtil.h" + +namespace ENetUtil +{ + +void WakeupThread(ENetHost* host) +{ + // Send ourselves a spurious message. This is hackier than it should be. + // comex reported this as https://github.com/lsalzman/enet/issues/23, so + // hopefully there will be a better way to do it in the future. + ENetAddress address; + if (host->address.port != 0) + address.port = host->address.port; + else + enet_socket_get_address(host->socket, &address); + address.host = 0x0100007f; // localhost + u8 byte = 0; + ENetBuffer buf; + buf.data = &byte; + buf.dataLength = 1; + enet_socket_send(host->socket, &address, &buf, 1); +} + +} diff --git a/Source/Core/Common/ENetUtil.h b/Source/Core/Common/ENetUtil.h new file mode 100644 index 0000000000..e9789673ad --- /dev/null +++ b/Source/Core/Common/ENetUtil.h @@ -0,0 +1,15 @@ +// Copyright 2015 Dolphin Emulator Project +// Licensed under GPLv2 +// Refer to the license.txt file included. +// +#pragma once + +#include +#include "Common.h" + +namespace ENetUtil +{ + +void WakeupThread(ENetHost* host); + +} diff --git a/Source/Core/Core/NetPlayClient.cpp b/Source/Core/Core/NetPlayClient.cpp index 2e792233fc..9aeaf3b8cf 100644 --- a/Source/Core/Core/NetPlayClient.cpp +++ b/Source/Core/Core/NetPlayClient.cpp @@ -485,25 +485,7 @@ void NetPlayClient::RunOnThread(std::function func) std::lock_guard lkq(m_crit.run_queue_write); m_run_queue.Push(func); } - WakeupThread(m_client); -} - -void NetPlayClient::WakeupThread(ENetHost* host) -{ - // Send ourselves a spurious message. This is hackier than it should be. - // comex reported this as https://github.com/lsalzman/enet/issues/23, so - // hopefully there will be a better way to do it in the future. - ENetAddress address; - if (host->address.port != 0) - address.port = host->address.port; - else - enet_socket_get_address(host->socket, &address); - address.host = 0x0100007f; // localhost - u8 byte = 0; - ENetBuffer buf; - buf.data = &byte; - buf.dataLength = 1; - enet_socket_send(host->socket, &address, &buf, 1); + ENetUtil::WakeupThread(m_client); } // called from ---NETPLAY--- thread diff --git a/Source/Core/Core/NetPlayClient.h b/Source/Core/Core/NetPlayClient.h index 7b2e3e159a..441cc17fb3 100644 --- a/Source/Core/Core/NetPlayClient.h +++ b/Source/Core/Core/NetPlayClient.h @@ -9,6 +9,7 @@ #include #include #include "Common/CommonTypes.h" +#include "Common/ENetUtil.h" #include "Common/FifoQueue.h" #include "Common/Thread.h" #include "Common/Timer.h" @@ -48,7 +49,6 @@ class NetPlayClient : public TraversalClientClient public: void ThreadFunc(); void RunOnThread(std::function func); - void WakeupThread(ENetHost* host); NetPlayClient(const std::string& address, const u16 port, NetPlayUI* dialog, const std::string& name, bool traversal, std::string centralServer, u16 centralPort); ~NetPlayClient(); diff --git a/Source/Core/Core/NetPlayServer.cpp b/Source/Core/Core/NetPlayServer.cpp index 7b96b14846..2c01b270f2 100644 --- a/Source/Core/Core/NetPlayServer.cpp +++ b/Source/Core/Core/NetPlayServer.cpp @@ -454,25 +454,7 @@ void NetPlayServer::RunOnThread(std::function func) std::lock_guard lkq(m_crit.run_queue_write); m_run_queue.Push(func); } - WakeupThread(m_server); -} - -void NetPlayServer::WakeupThread(ENetHost* host) -{ - // Send ourselves a spurious message. This is hackier than it should be. - // comex reported this as https://github.com/lsalzman/enet/issues/23, so - // hopefully there will be a better way to do it in the future. - ENetAddress address; - if (host->address.port != 0) - address.port = host->address.port; - else - enet_socket_get_address(host->socket, &address); - address.host = 0x0100007f; // localhost - u8 byte = 0; - ENetBuffer buf; - buf.data = &byte; - buf.dataLength = 1; - enet_socket_send(host->socket, &address, &buf, 1); + ENetUtil::WakeupThread(m_server); } // called from ---NETPLAY--- thread diff --git a/Source/Core/Core/NetPlayServer.h b/Source/Core/Core/NetPlayServer.h index 21f7ea9ede..6c53b537ad 100644 --- a/Source/Core/Core/NetPlayServer.h +++ b/Source/Core/Core/NetPlayServer.h @@ -9,6 +9,7 @@ #include #include #include +#include "Common/ENetUtil.h" #include "Common/Thread.h" #include "Common/Timer.h" #include "Common/TraversalClient.h" @@ -21,7 +22,6 @@ class NetPlayServer : public TraversalClientClient public: void ThreadFunc(); void RunOnThread(std::function func); - void WakeupThread(ENetHost* host); NetPlayServer(const u16 port, bool traversal, std::string centralServer, u16 centralPort); ~NetPlayServer(); From 8ee402863dbbd19b012c4653e0e9e54711cfe35f Mon Sep 17 00:00:00 2001 From: mathieui Date: Fri, 13 Mar 2015 02:03:09 +0100 Subject: [PATCH 3/6] NetPlay: Remove RunOnThread and add SendAsync methods Add std::unique_ptr objects to a queue instead of functions, makes things easier to read, and avoids headaches while checking the lifetime of the concerned objects. --- Source/Core/Core/NetPlayClient.cpp | 80 ++++++++++++---------------- Source/Core/Core/NetPlayClient.h | 6 +-- Source/Core/Core/NetPlayServer.cpp | 83 +++++++++++++----------------- Source/Core/Core/NetPlayServer.h | 6 +-- 4 files changed, 77 insertions(+), 98 deletions(-) diff --git a/Source/Core/Core/NetPlayClient.cpp b/Source/Core/Core/NetPlayClient.cpp index 9aeaf3b8cf..cbc5b26981 100644 --- a/Source/Core/Core/NetPlayClient.cpp +++ b/Source/Core/Core/NetPlayClient.cpp @@ -479,11 +479,11 @@ void NetPlayClient::Disconnect() m_server = nullptr; } -void NetPlayClient::RunOnThread(std::function func) +void NetPlayClient::SendAsync(sf::Packet* packet) { { - std::lock_guard lkq(m_crit.run_queue_write); - m_run_queue.Push(func); + std::lock_guard lkq(m_crit.async_queue_write); + m_async_queue.Push(std::unique_ptr(packet)); } ENetUtil::WakeupThread(m_client); } @@ -498,10 +498,10 @@ void NetPlayClient::ThreadFunc() if (m_traversal_client) m_traversal_client->HandleResends(); net = enet_host_service(m_client, &netEvent, 4); - while (!m_run_queue.Empty()) + while (!m_async_queue.Empty()) { - m_run_queue.Front()(); - m_run_queue.Pop(); + Send(*(m_async_queue.Front().get())); + m_async_queue.Pop(); } if (net > 0) { @@ -586,57 +586,47 @@ void NetPlayClient::GetPlayers(std::vector &player_list) // called from ---GUI--- thread void NetPlayClient::SendChatMessage(const std::string& msg) { - RunOnThread([msg, this]() { - sf::Packet spac; - spac << (MessageId)NP_MSG_CHAT_MESSAGE; - spac << msg; - Send(spac); - }); + sf::Packet* spac = new sf::Packet; + *spac << (MessageId)NP_MSG_CHAT_MESSAGE; + *spac << msg; + SendAsync(spac); } // called from ---CPU--- thread void NetPlayClient::SendPadState(const PadMapping in_game_pad, const GCPadStatus& pad) { - sf::Packet spac; - spac << (MessageId)NP_MSG_PAD_DATA; - spac << in_game_pad; - spac << pad.button << pad.analogA << pad.analogB << pad.stickX << pad.stickY << pad.substickX << pad.substickY << pad.triggerLeft << pad.triggerRight; + sf::Packet* spac = new sf::Packet; + *spac << (MessageId)NP_MSG_PAD_DATA; + *spac << in_game_pad; + *spac << pad.button << pad.analogA << pad.analogB << pad.stickX << pad.stickY << pad.substickX << pad.substickY << pad.triggerLeft << pad.triggerRight; - RunOnThread([spac, this]() mutable { - // send to server - Send(spac); - }); + SendAsync(spac); } // called from ---CPU--- thread void NetPlayClient::SendWiimoteState(const PadMapping in_game_pad, const NetWiimote& nw) { - RunOnThread([=]() { - // send to server - sf::Packet spac; - spac << (MessageId)NP_MSG_WIIMOTE_DATA; - spac << in_game_pad; - spac << (u8)nw.size(); - for (auto it : nw) - { - spac << it; - } - Send(spac); - }); + sf::Packet* spac = new sf::Packet; + *spac << (MessageId)NP_MSG_WIIMOTE_DATA; + *spac << in_game_pad; + *spac << (u8)nw.size(); + for (auto it : nw) + { + *spac << it; + } + SendAsync(spac); } // called from ---GUI--- thread bool NetPlayClient::StartGame(const std::string &path) { - RunOnThread([this](){ - std::lock_guard lkg(m_crit.game); - // tell server i started the game - sf::Packet spac; - spac << (MessageId)NP_MSG_START_GAME; - spac << m_current_game; - spac << (char *)&g_NetPlaySettings; - Send(spac); - }); + std::lock_guard lkg(m_crit.game); + // tell server i started the game + sf::Packet* spac = new sf::Packet; + *spac << (MessageId)NP_MSG_START_GAME; + *spac << m_current_game; + *spac << (char *)&g_NetPlaySettings; + SendAsync(spac); if (m_is_running) { @@ -986,11 +976,9 @@ void NetPlayClient::Stop() // tell the server to stop if we have a pad mapped in game. if (isPadMapped) { - RunOnThread([this](){ - sf::Packet spac; - spac << (MessageId)NP_MSG_STOP_GAME; - Send(spac); - }); + sf::Packet* spac = new sf::Packet; + *spac << (MessageId)NP_MSG_STOP_GAME; + SendAsync(spac); } } diff --git a/Source/Core/Core/NetPlayClient.h b/Source/Core/Core/NetPlayClient.h index 441cc17fb3..e47dcb7e94 100644 --- a/Source/Core/Core/NetPlayClient.h +++ b/Source/Core/Core/NetPlayClient.h @@ -48,7 +48,7 @@ class NetPlayClient : public TraversalClientClient { public: void ThreadFunc(); - void RunOnThread(std::function func); + void SendAsync(sf::Packet* packet); NetPlayClient(const std::string& address, const u16 port, NetPlayUI* dialog, const std::string& name, bool traversal, std::string centralServer, u16 centralPort); ~NetPlayClient(); @@ -95,10 +95,10 @@ protected: std::recursive_mutex game; // lock order std::recursive_mutex players; - std::recursive_mutex run_queue_write; + std::recursive_mutex async_queue_write; } m_crit; - Common::FifoQueue, false> m_run_queue; + Common::FifoQueue, false> m_async_queue; Common::FifoQueue m_pad_buffer[4]; Common::FifoQueue m_wiimote_buffer[4]; diff --git a/Source/Core/Core/NetPlayServer.cpp b/Source/Core/Core/NetPlayServer.cpp index 2c01b270f2..499a6088cb 100644 --- a/Source/Core/Core/NetPlayServer.cpp +++ b/Source/Core/Core/NetPlayServer.cpp @@ -126,10 +126,13 @@ void NetPlayServer::ThreadFunc() if (m_traversal_client) m_traversal_client->HandleResends(); net = enet_host_service(m_server, &netEvent, 1000); - while (!m_run_queue.Empty()) + while (!m_async_queue.Empty()) { - m_run_queue.Front()(); - m_run_queue.Pop(); + { + std::lock_guard lkp(m_crit.players); + SendToClients(*(m_async_queue.Front().get())); + } + m_async_queue.Pop(); } if (net > 0) { @@ -438,21 +441,18 @@ void NetPlayServer::AdjustPadBufferSize(unsigned int size) m_target_buffer_size = size; // tell clients to change buffer size - sf::Packet spac; - spac << (MessageId)NP_MSG_PAD_BUFFER; - spac << (u32)m_target_buffer_size; + sf::Packet* spac = new sf::Packet; + *spac << (MessageId)NP_MSG_PAD_BUFFER; + *spac << (u32)m_target_buffer_size; - RunOnThread([spac, this]() mutable { - std::lock_guard lkp(m_crit.players); - SendToClients(spac); - }); + SendAsyncToClients(spac); } -void NetPlayServer::RunOnThread(std::function func) +void NetPlayServer::SendAsyncToClients(sf::Packet* packet) { { - std::lock_guard lkq(m_crit.run_queue_write); - m_run_queue.Push(func); + std::lock_guard lkq(m_crit.async_queue_write); + m_async_queue.Push(std::unique_ptr(packet)); } ENetUtil::WakeupThread(m_server); } @@ -598,15 +598,12 @@ void NetPlayServer::OnTraversalStateChanged() // called from ---GUI--- thread void NetPlayServer::SendChatMessage(const std::string& msg) { - sf::Packet spac; - spac << (MessageId)NP_MSG_CHAT_MESSAGE; - spac << (PlayerId)0; // server id always 0 - spac << msg; + sf::Packet* spac = new sf::Packet; + *spac << (MessageId)NP_MSG_CHAT_MESSAGE; + *spac << (PlayerId)0; // server id always 0 + *spac << msg; - RunOnThread([spac, this]() mutable { - std::lock_guard lkp(m_crit.players); - SendToClients(spac); - }); + SendAsyncToClients(spac); } // called from ---GUI--- thread @@ -617,14 +614,11 @@ bool NetPlayServer::ChangeGame(const std::string &game) m_selected_game = game; // send changed game to clients - sf::Packet spac; - spac << (MessageId)NP_MSG_CHANGE_GAME; - spac << game; + sf::Packet* spac = new sf::Packet; + *spac << (MessageId)NP_MSG_CHANGE_GAME; + *spac << game; - RunOnThread([spac, this]() mutable { - std::lock_guard lkp(m_crit.players); - SendToClients(spac); - }); + SendAsyncToClients(spac); return true; } @@ -647,25 +641,22 @@ bool NetPlayServer::StartGame() g_netplay_initial_gctime = Common::Timer::GetLocalTimeSinceJan1970(); // tell clients to start game - sf::Packet spac; - spac << (MessageId)NP_MSG_START_GAME; - spac << m_current_game; - spac << m_settings.m_CPUthread; - spac << m_settings.m_CPUcore; - spac << m_settings.m_DSPEnableJIT; - spac << m_settings.m_DSPHLE; - spac << m_settings.m_WriteToMemcard; - spac << m_settings.m_OCEnable; - spac << m_settings.m_OCFactor; - spac << m_settings.m_EXIDevice[0]; - spac << m_settings.m_EXIDevice[1]; - spac << (u32)g_netplay_initial_gctime; - spac << (u32)g_netplay_initial_gctime << 32; + sf::Packet* spac = new sf::Packet; + *spac << (MessageId)NP_MSG_START_GAME; + *spac << m_current_game; + *spac << m_settings.m_CPUthread; + *spac << m_settings.m_CPUcore; + *spac << m_settings.m_DSPEnableJIT; + *spac << m_settings.m_DSPHLE; + *spac << m_settings.m_WriteToMemcard; + *spac << m_settings.m_OCEnable; + *spac << m_settings.m_OCFactor; + *spac << m_settings.m_EXIDevice[0]; + *spac << m_settings.m_EXIDevice[1]; + *spac << (u32)g_netplay_initial_gctime; + *spac << (u32)g_netplay_initial_gctime << 32; - RunOnThread([spac, this]() mutable { - std::lock_guard lkp(m_crit.players); - SendToClients(spac); - }); + SendAsyncToClients(spac); m_is_running = true; diff --git a/Source/Core/Core/NetPlayServer.h b/Source/Core/Core/NetPlayServer.h index 6c53b537ad..e1a22c8de9 100644 --- a/Source/Core/Core/NetPlayServer.h +++ b/Source/Core/Core/NetPlayServer.h @@ -21,7 +21,7 @@ class NetPlayServer : public TraversalClientClient { public: void ThreadFunc(); - void RunOnThread(std::function func); + void SendAsyncToClients(sf::Packet* packet); NetPlayServer(const u16 port, bool traversal, std::string centralServer, u16 centralPort); ~NetPlayServer(); @@ -104,12 +104,12 @@ private: std::recursive_mutex game; // lock order std::recursive_mutex players; - std::recursive_mutex run_queue_write; + std::recursive_mutex async_queue_write; } m_crit; std::string m_selected_game; std::thread m_thread; - Common::FifoQueue, false> m_run_queue; + Common::FifoQueue, false> m_async_queue; ENetHost* m_server; TraversalClient* m_traversal_client; From e0ef8fc03f1fafc801ca5d857347f9fa1ea58881 Mon Sep 17 00:00:00 2001 From: mathieui Date: Sat, 14 Mar 2015 15:19:18 +0100 Subject: [PATCH 4/6] NetPlay: Make the enet interrupts work Otherwise, it would work but any async sending would be delayed by 4ms or wait until the next packet was received. Also increase the client timeout to 250ms, since enet_host_service is now really interrupted. --- Source/Core/Common/ENetUtil.cpp | 12 ++++++++++++ Source/Core/Common/ENetUtil.h | 1 + Source/Core/Common/TraversalClient.cpp | 3 ++- Source/Core/Core/NetPlayClient.cpp | 5 ++++- Source/Core/Core/NetPlayServer.cpp | 2 ++ 5 files changed, 21 insertions(+), 2 deletions(-) diff --git a/Source/Core/Common/ENetUtil.cpp b/Source/Core/Common/ENetUtil.cpp index 7269893f90..ae5b19b2c3 100644 --- a/Source/Core/Common/ENetUtil.cpp +++ b/Source/Core/Common/ENetUtil.cpp @@ -25,4 +25,16 @@ void WakeupThread(ENetHost* host) enet_socket_send(host->socket, &address, &buf, 1); } +int ENET_CALLBACK InterceptCallback(ENetHost* host, ENetEvent* event) +{ + // wakeup packet received + if (host->receivedDataLength == 1 && host->receivedData[0] == 0) + { + event->type = (ENetEventType) 42; + return 1; + } + return 0; +} + + } diff --git a/Source/Core/Common/ENetUtil.h b/Source/Core/Common/ENetUtil.h index e9789673ad..219f51d425 100644 --- a/Source/Core/Common/ENetUtil.h +++ b/Source/Core/Common/ENetUtil.h @@ -11,5 +11,6 @@ namespace ENetUtil { void WakeupThread(ENetHost* host); +int ENET_CALLBACK InterceptCallback(ENetHost* host, ENetEvent* event); } diff --git a/Source/Core/Common/TraversalClient.cpp b/Source/Core/Common/TraversalClient.cpp index 72d18244f3..02fe5ee26e 100644 --- a/Source/Core/Common/TraversalClient.cpp +++ b/Source/Core/Common/TraversalClient.cpp @@ -301,7 +301,8 @@ void TraversalClient::Reset() int ENET_CALLBACK TraversalClient::InterceptCallback(ENetHost* host, ENetEvent* event) { auto traversalClient = g_TraversalClient.get(); - if (traversalClient->TestPacket(host->receivedData, host->receivedDataLength, &host->receivedAddress)) + if (traversalClient->TestPacket(host->receivedData, host->receivedDataLength, &host->receivedAddress) + || (host->receivedDataLength == 1 && host->receivedData[0] == 0)) { event->type = (ENetEventType)42; return 1; diff --git a/Source/Core/Core/NetPlayClient.cpp b/Source/Core/Core/NetPlayClient.cpp index cbc5b26981..b424932e77 100644 --- a/Source/Core/Core/NetPlayClient.cpp +++ b/Source/Core/Core/NetPlayClient.cpp @@ -103,7 +103,10 @@ NetPlayClient::NetPlayClient(const std::string& address, const u16 port, NetPlay if (net > 0 && netEvent.type == ENET_EVENT_TYPE_CONNECT) { if (Connect()) + { + m_client->intercept = ENetUtil::InterceptCallback; m_thread = std::thread(&NetPlayClient::ThreadFunc, this); + } } else { @@ -497,7 +500,7 @@ void NetPlayClient::ThreadFunc() int net; if (m_traversal_client) m_traversal_client->HandleResends(); - net = enet_host_service(m_client, &netEvent, 4); + net = enet_host_service(m_client, &netEvent, 250); while (!m_async_queue.Empty()) { Send(*(m_async_queue.Front().get())); diff --git a/Source/Core/Core/NetPlayServer.cpp b/Source/Core/Core/NetPlayServer.cpp index 499a6088cb..977d87e1fc 100644 --- a/Source/Core/Core/NetPlayServer.cpp +++ b/Source/Core/Core/NetPlayServer.cpp @@ -91,6 +91,8 @@ NetPlayServer::NetPlayServer(const u16 port, bool traversal, std::string central serverAddr.host = ENET_HOST_ANY; serverAddr.port = port; m_server = enet_host_create(&serverAddr, 10, 3, 0, 0); + if (m_server != nullptr) + m_server->intercept = ENetUtil::InterceptCallback; } if (m_server != nullptr) { From 849922881dbac56de584d079d65e14b2747df9af Mon Sep 17 00:00:00 2001 From: mathieui Date: Sun, 15 Mar 2015 21:19:31 +0100 Subject: [PATCH 5/6] NetPlay: Select a game only if enet connected successfully --- Source/Core/DolphinWX/NetWindow.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Source/Core/DolphinWX/NetWindow.cpp b/Source/Core/DolphinWX/NetWindow.cpp index 02e16cda3b..bae82f5bec 100644 --- a/Source/Core/DolphinWX/NetWindow.cpp +++ b/Source/Core/DolphinWX/NetWindow.cpp @@ -360,10 +360,10 @@ void NetPlaySetupDiag::OnHost(wxCommandEvent&) unsigned long centralPort = 0; m_traversal_port->GetValue().ToULong(¢ralPort); netplay_server = new NetPlayServer(u16(port), trav, WxStrToStr(m_traversal_server->GetValue()), u16(centralPort)); - netplay_server->ChangeGame(game); - netplay_server->AdjustPadBufferSize(INITIAL_PAD_BUFFER_SIZE); if (netplay_server->is_connected) { + netplay_server->ChangeGame(game); + netplay_server->AdjustPadBufferSize(INITIAL_PAD_BUFFER_SIZE); #ifdef USE_UPNP if (m_upnp_chk->GetValue()) netplay_server->TryPortmapping(port); From 8201a52cec975c6ed9c81bd6f33c41b3ef7b473a Mon Sep 17 00:00:00 2001 From: mathieui Date: Tue, 10 Mar 2015 01:47:29 +0100 Subject: [PATCH 6/6] Traversal: Use a decent PRNG instead of rand() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit we don’t need cryptosecure random, but having a uniform distribution is always better. --- Source/Core/Common/TraversalClient.cpp | 5 +++-- Source/Core/Common/TraversalClient.h | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Source/Core/Common/TraversalClient.cpp b/Source/Core/Common/TraversalClient.cpp index 02fe5ee26e..f17ce0d921 100644 --- a/Source/Core/Common/TraversalClient.cpp +++ b/Source/Core/Common/TraversalClient.cpp @@ -7,9 +7,10 @@ static void GetRandomishBytes(u8* buf, size_t size) { // We don't need high quality random numbers (which might not be available), // just non-repeating numbers! - srand(enet_time_get()); + static std::mt19937 prng(enet_time_get()); + static std::uniform_int_distribution u8_distribution(0, 255); for (size_t i = 0; i < size; i++) - buf[i] = rand() & 0xff; + buf[i] = u8_distribution(prng); } TraversalClient::TraversalClient(ENetHost* netHost, const std::string& server, const u16 port) diff --git a/Source/Core/Common/TraversalClient.h b/Source/Core/Common/TraversalClient.h index 0263d863fb..ddfb38fce1 100644 --- a/Source/Core/Common/TraversalClient.h +++ b/Source/Core/Common/TraversalClient.h @@ -4,6 +4,7 @@ #include #include #include +#include #include #include "Common/Common.h" #include "Common/Thread.h"