From dd4b70a24b9094c95d0fc9a5fe5cd583d73927e0 Mon Sep 17 00:00:00 2001 From: Vladimir Olteanu Date: Sat, 17 Jul 2021 16:03:51 +0300 Subject: [PATCH] * dthread.cpp: replace TMegaPkt with DThreadStruct * consistently use uint8_t for pnum * get rid of checks that never fail * use atomic_bool for dthread_running * dthread: eliminate superfluous memory copies * Pass correct-sized buffer to DeltaExportJunk * dthread: Fix crash when list is empty * DthreadHandler: eliminate delay * Dthread: use SdlMutex and lock_guard * dhtread: explicitly provide T for lock_guard --- Source/dthread.cpp | 129 ++++++++++++++++----------------------------- Source/dthread.h | 5 +- Source/msg.cpp | 23 ++++---- Source/multi.cpp | 8 +-- 4 files changed, 67 insertions(+), 98 deletions(-) diff --git a/Source/dthread.cpp b/Source/dthread.cpp index 587472445..a85913afc 100644 --- a/Source/dthread.cpp +++ b/Source/dthread.cpp @@ -4,18 +4,37 @@ * Implementation of functions for updating game state from network commands. */ +#include +#include +#include + #include "nthread.h" -#include "storm/storm.h" +#include "utils/sdl_mutex.h" #include "utils/thread.h" namespace devilution { +struct DThreadPkt { + int pnum; + _cmd_id cmd; + std::unique_ptr data; + uint32_t len; + + DThreadPkt(int pnum, _cmd_id(cmd), std::unique_ptr data, uint32_t len) + : pnum(pnum) + , cmd(cmd) + , data(std::move(data)) + , len(len) + { + } +}; + namespace { -CCritSect sgMemCrit; +SdlMutex DthreadMutex; SDL_threadID glpDThreadId; -TMegaPkt *sgpInfoHead; /* may not be right struct */ -bool dthread_running; +std::list InfoList; +std::atomic_bool dthread_running; event_emul *sghWorkToDoEvent; /* rdata */ @@ -23,37 +42,21 @@ SDL_Thread *sghThread = nullptr; void DthreadHandler() { - const char *errorBuf; - TMegaPkt *pkt; - DWORD dwMilliseconds; - while (dthread_running) { - if (sgpInfoHead == nullptr && WaitForEvent(sghWorkToDoEvent) == -1) { - errorBuf = SDL_GetError(); - app_fatal("dthread4:\n%s", errorBuf); - } + if (InfoList.empty() && WaitForEvent(sghWorkToDoEvent) == -1) + app_fatal("dthread4:\n%s", SDL_GetError()); - sgMemCrit.Enter(); - pkt = sgpInfoHead; - if (sgpInfoHead != nullptr) - sgpInfoHead = sgpInfoHead->pNext; - else + DthreadMutex.lock(); + if (InfoList.empty()) { ResetEvent(sghWorkToDoEvent); - sgMemCrit.Leave(); - - if (pkt != nullptr) { - if (pkt->dwSpaceLeft != MAX_PLRS) - multi_send_zero_packet(pkt->dwSpaceLeft, static_cast<_cmd_id>(pkt->data[0]), &pkt->data[8], *(DWORD *)&pkt->data[4]); - - dwMilliseconds = 1000 * *(DWORD *)&pkt->data[4] / gdwDeltaBytesSec; - if (dwMilliseconds >= 1) - dwMilliseconds = 1; - - std::free(pkt); - - if (dwMilliseconds != 0) - SDL_Delay(dwMilliseconds); + DthreadMutex.unlock(); + continue; } + DThreadPkt pkt = std::move(InfoList.front()); + InfoList.pop_front(); + DthreadMutex.unlock(); + + multi_send_zero_packet(pkt.pnum, pkt.cmd, pkt.data.get(), pkt.len); } } @@ -61,74 +64,38 @@ void DthreadHandler() void dthread_remove_player(uint8_t pnum) { - TMegaPkt *pkt; - - sgMemCrit.Enter(); - for (pkt = sgpInfoHead; pkt != nullptr; pkt = pkt->pNext) { - if (pkt->dwSpaceLeft == pnum) - pkt->dwSpaceLeft = MAX_PLRS; - } - sgMemCrit.Leave(); + std::lock_guard lock(DthreadMutex); + InfoList.remove_if([&](auto &pkt) { + return pkt.pnum == pnum; + }); } -void dthread_send_delta(int pnum, _cmd_id cmd, byte *pbSrc, int dwLen) +void dthread_send_delta(int pnum, _cmd_id cmd, std::unique_ptr data, uint32_t len) { - TMegaPkt *pkt; - TMegaPkt *p; - - if (!gbIsMultiplayer) { + if (!gbIsMultiplayer) return; - } - pkt = static_cast(std::malloc(dwLen + 20)); - if (pkt == nullptr) - app_fatal("Failed to allocate memory"); - pkt->pNext = nullptr; - pkt->dwSpaceLeft = pnum; - pkt->data[0] = static_cast(cmd); - *(DWORD *)&pkt->data[4] = dwLen; - memcpy(&pkt->data[8], pbSrc, dwLen); - sgMemCrit.Enter(); - p = (TMegaPkt *)&sgpInfoHead; - while (p->pNext != nullptr) { - p = p->pNext; - } - p->pNext = pkt; + DThreadPkt pkt { pnum, cmd, std::move(data), len }; + std::lock_guard lock(DthreadMutex); + InfoList.push_back(std::move(pkt)); SetEvent(sghWorkToDoEvent); - sgMemCrit.Leave(); } void dthread_start() { - const char *errorBuf; - - if (!gbIsMultiplayer) { + if (!gbIsMultiplayer) return; - } sghWorkToDoEvent = StartEvent(); - if (sghWorkToDoEvent == nullptr) { - errorBuf = SDL_GetError(); - app_fatal("dthread:1\n%s", errorBuf); - } - dthread_running = true; - sghThread = CreateThread(DthreadHandler, &glpDThreadId); - if (sghThread == nullptr) { - errorBuf = SDL_GetError(); - app_fatal("dthread2:\n%s", errorBuf); - } } void DThreadCleanup() { - TMegaPkt *tmp; - - if (sghWorkToDoEvent == nullptr) { + if (sghWorkToDoEvent == nullptr) return; - } dthread_running = false; SetEvent(sghWorkToDoEvent); @@ -139,11 +106,7 @@ void DThreadCleanup() EndEvent(sghWorkToDoEvent); sghWorkToDoEvent = nullptr; - while (sgpInfoHead != nullptr) { - tmp = sgpInfoHead->pNext; - std::free(sgpInfoHead); - sgpInfoHead = tmp; - } + InfoList.clear(); } } // namespace devilution diff --git a/Source/dthread.h b/Source/dthread.h index d6a5f573d..ec812f26b 100644 --- a/Source/dthread.h +++ b/Source/dthread.h @@ -5,10 +5,13 @@ */ #pragma once +#include +#include "utils/stdcompat/cstddef.hpp" + namespace devilution { void dthread_remove_player(uint8_t pnum); -void dthread_send_delta(int pnum, _cmd_id cmd, byte *pbSrc, int dwLen); +void dthread_send_delta(int pnum, _cmd_id cmd, std::unique_ptr data, uint32_t len); void dthread_start(); void DThreadCleanup(); diff --git a/Source/msg.cpp b/Source/msg.cpp index 1b3664263..54821dfbd 100644 --- a/Source/msg.cpp +++ b/Source/msg.cpp @@ -1863,24 +1863,25 @@ void run_delta_info() void DeltaExportData(int pnum) { if (sgbDeltaChanged) { - int size; - std::unique_ptr dst { new byte[sizeof(DLevel) + 1] }; - byte *dstEnd; for (int i = 0; i < NUMLEVELS; i++) { - dstEnd = &dst[1]; + std::unique_ptr dst { new byte[sizeof(DLevel) + 1] }; + byte *dstEnd = &dst.get()[1]; dstEnd = DeltaExportItem(dstEnd, sgLevels[i].item); dstEnd = DeltaExportObject(dstEnd, sgLevels[i].object); dstEnd = DeltaExportMonster(dstEnd, sgLevels[i].monster); - size = CompressData(dst.get(), dstEnd); - dthread_send_delta(pnum, static_cast<_cmd_id>(i + CMD_DLEVEL_0), dst.get(), size); + int size = CompressData(dst.get(), dstEnd); + dthread_send_delta(pnum, static_cast<_cmd_id>(i + CMD_DLEVEL_0), std::move(dst), size); } - dstEnd = &dst[1]; + + std::unique_ptr dst { new byte[sizeof(DJunk) + 1] }; + byte *dstEnd = &dst.get()[1]; dstEnd = DeltaExportJunk(dstEnd); - size = CompressData(dst.get(), dstEnd); - dthread_send_delta(pnum, CMD_DLEVEL_JUNK, dst.get(), size); + int size = CompressData(dst.get(), dstEnd); + dthread_send_delta(pnum, CMD_DLEVEL_JUNK, std::move(dst), size); } - byte src { 0 }; - dthread_send_delta(pnum, CMD_DLEVEL_END, &src, 1); + + std::unique_ptr src { new byte[1] { static_cast(0) } }; + dthread_send_delta(pnum, CMD_DLEVEL_END, std::move(src), 1); } void delta_init() diff --git a/Source/multi.cpp b/Source/multi.cpp index 85426e14c..a14b988a3 100644 --- a/Source/multi.cpp +++ b/Source/multi.cpp @@ -24,6 +24,7 @@ #include "tmsg.h" #include "utils/endian.hpp" #include "utils/language.h" +#include "utils/stdcompat/cstddef.hpp" namespace devilution { @@ -311,10 +312,11 @@ void ProcessTmsgs() void SendPlayerInfo(int pnum, _cmd_id cmd) { - PkPlayerStruct pkplr; + static_assert(alignof(PkPlayerStruct) == 1, "Fix pkplr alignment"); + std::unique_ptr pkplr { new byte[sizeof(PkPlayerStruct)] }; - PackPlayer(&pkplr, Players[MyPlayerId], true); - dthread_send_delta(pnum, cmd, (byte *)&pkplr, sizeof(pkplr)); + PackPlayer(reinterpret_cast(pkplr.get()), Players[MyPlayerId], true); + dthread_send_delta(pnum, cmd, std::move(pkplr), sizeof(PkPlayerStruct)); } dungeon_type InitLevelType(int l)