From 5244538b77a1832d9cdc6fe03af9e87178a8f419 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 28 Oct 2024 16:59:53 +0100 Subject: [PATCH] Reporting: Delete the now unused extra threads, fix up compat reports. --- Common/LogReporting.cpp | 4 +- Core/Reporting.cpp | 159 ++++++++-------------------------------- UI/ReportScreen.cpp | 2 +- 3 files changed, 35 insertions(+), 130 deletions(-) diff --git a/Common/LogReporting.cpp b/Common/LogReporting.cpp index 7c6ac3b440..9f41875aa0 100644 --- a/Common/LogReporting.cpp +++ b/Common/LogReporting.cpp @@ -67,7 +67,7 @@ void ReportMessage(const char *message, ...) { return; const int MESSAGE_BUFFER_SIZE = 65536; - char temp[MESSAGE_BUFFER_SIZE]; + char *temp = new char [MESSAGE_BUFFER_SIZE]; va_list args; va_start(args, message); @@ -76,6 +76,8 @@ void ReportMessage(const char *message, ...) { va_end(args); messageCallback(message, temp); + + delete[] temp; } void ReportMessageFormatted(const char *message, const char *formatted) { diff --git a/Core/Reporting.cpp b/Core/Reporting.cpp index 75a4770475..d49ab0cdb5 100644 --- a/Core/Reporting.cpp +++ b/Core/Reporting.cpp @@ -84,22 +84,13 @@ namespace Reporting static int lastModuleVersion; static uint32_t lastModuleCrc; - static std::mutex pendingMessageLock; - static std::condition_variable pendingMessageCond; - static std::deque pendingMessages; - static bool pendingMessagesDone = false; - static std::thread messageThread; - static std::thread compatThread; - - enum class RequestType - { + enum class RequestType { NONE, MESSAGE, COMPAT, }; - struct Payload - { + struct Payload { RequestType type; std::string string1; std::string string2; @@ -107,8 +98,6 @@ namespace Reporting int int2; int int3; }; - static Payload payloadBuffer[PAYLOAD_BUFFER_SIZE]; - static int payloadBufferPos = 0; static std::mutex crcLock; static std::condition_variable crcCond; @@ -240,8 +229,7 @@ namespace Reporting } // Returns the full host (e.g. report.ppsspp.org:80.) - std::string ServerHost() - { + std::string ServerHost() { if (g_Config.sReportHost.compare("default") == 0) return ""; return g_Config.sReportHost; @@ -306,20 +294,15 @@ namespace Reporting return ++spamProtectionCount >= SPAM_LIMIT; } - bool SendReportRequest(const char *uri, const std::string &data, const std::string &mimeType, Buffer *output = NULL) - { + static void SendReportRequest(const char *uri, const std::string &data, const std::string &mimeType, std::function callback) { char url[1024]; const char *hostname = ServerHostname(); int port = ServerPort(); snprintf(url, sizeof(url), "http://%s:%d%s", hostname, port, uri); - g_DownloadManager.AsyncPostWithCallback(url, data, mimeType, http::ProgressBarMode::NONE, [=](http::Request &req) { - serverWorking = !req.Failed(); - }); - return true; + g_DownloadManager.AsyncPostWithCallback(url, data, mimeType, http::ProgressBarMode::NONE, callback); } - std::string StripTrailingNull(const std::string &str) - { + std::string StripTrailingNull(const std::string &str) { size_t pos = str.find_first_of('\0'); if (pos != str.npos) return str.substr(0, pos); @@ -369,14 +352,12 @@ namespace Reporting bool MessageAllowed(); void SendReportMessage(const char *message, const char *formatted); - void Init() - { + void Init() { // New game, clean slate. spamProtectionCount = 0; ResetCounts(); everUnsupported = false; currentSupported = IsSupported(); - pendingMessagesDone = false; Reporting::SetupCallbacks(&MessageAllowed, &SendReportMessage); lastModuleName.clear(); @@ -384,24 +365,14 @@ namespace Reporting lastModuleCrc = 0; } - void Shutdown() - { - pendingMessageLock.lock(); - pendingMessagesDone = true; - pendingMessageCond.notify_one(); - pendingMessageLock.unlock(); - if (compatThread.joinable()) - compatThread.join(); - if (messageThread.joinable()) - messageThread.join(); + void Shutdown() { PurgeCRC(); // Just so it can be enabled in the menu again. Init(); } - void DoState(PointerWrap &p) - { + void DoState(PointerWrap &p) { const int LATEST_VERSION = 1; auto s = p.Section("Reporting", 0, LATEST_VERSION); if (!s || s < LATEST_VERSION) { @@ -413,8 +384,7 @@ namespace Reporting Do(p, everUnsupported); } - void UpdateConfig() - { + void UpdateConfig() { currentSupported = IsSupported(); if (!currentSupported && PSP_IsInited()) everUnsupported = true; @@ -498,9 +468,7 @@ namespace Reporting } } - // Not the thread func, but called from it. - int Process(int pos) { - Payload &payload = payloadBuffer[pos]; + int Process(const Payload &payload) { Buffer output; MultipartFormDataEncoder postdata; @@ -509,21 +477,18 @@ namespace Reporting AddConfigInfo(postdata); AddGameplayInfo(postdata); - switch (payload.type) - { + switch (payload.type) { case RequestType::MESSAGE: // TODO: Add CRC? postdata.Add("message", payload.string1); postdata.Add("value", payload.string2); // We tend to get corrupted data, this acts as a very primitive verification check. postdata.Add("verify", payload.string1 + payload.string2); - payload.string1.clear(); - payload.string2.clear(); postdata.Finish(); - serverWorking = true; - if (!SendReportRequest("/report/message", postdata.ToString(), postdata.GetMimeType())) - serverWorking = false; + SendReportRequest("/report/message", postdata.ToString(), postdata.GetMimeType(), [=](http::Request &req) { + serverWorking = !req.Failed(); + }); break; case RequestType::COMPAT: @@ -536,31 +501,28 @@ namespace Reporting postdata.Add("crc", StringFromFormat("%08x", RetrieveCRCUnlessPowerSaving(PSP_CoreParameter().fileToStart))); postdata.Add("suggestions", payload.string1 != "perfect" && payload.string1 != "playable" ? "1" : "0"); AddScreenshotData(postdata, Path(payload.string2)); - payload.string1.clear(); - payload.string2.clear(); postdata.Finish(); serverWorking = true; - if (!SendReportRequest("/report/compat", postdata.ToString(), postdata.GetMimeType(), &output)) { - serverWorking = false; - } else { - std::string result; - output.TakeAll(&result); + SendReportRequest("/report/compat", postdata.ToString(), postdata.GetMimeType(), [=](http::Request &req) { + if (req.Failed()) { + serverWorking = false; + return; + } + serverWorking = true; + std::string result; + req.buffer().TakeAll(&result); lastCompatResult.clear(); if (result.empty() || result[0] == '0') serverWorking = false; else if (result[0] != '1') SplitString(result, '\n', lastCompatResult); - } + }); break; - case RequestType::NONE: break; } - - payload.type = RequestType::NONE; - return 0; } @@ -623,56 +585,12 @@ namespace Reporting g_Config.sReportHost = "default"; } - ReportStatus GetStatus() - { + ReportStatus GetStatus() { if (!serverWorking) return ReportStatus::FAILING; - - for (int pos = 0; pos < PAYLOAD_BUFFER_SIZE; ++pos) - { - if (payloadBuffer[pos].type != RequestType::NONE) - return ReportStatus::BUSY; - } - return ReportStatus::WORKING; } - int NextFreePos() - { - int start = payloadBufferPos % PAYLOAD_BUFFER_SIZE; - do - { - int pos = payloadBufferPos++ % PAYLOAD_BUFFER_SIZE; - if (payloadBuffer[pos].type == RequestType::NONE) - return pos; - } - while (payloadBufferPos != start); - - return -1; - } - - int SendPendingReportsThread() { - SetCurrentThreadName("Report"); - - std::unique_lock guard(pendingMessageLock); - while (!pendingMessagesDone) { - while (!pendingMessages.empty() && !pendingMessagesDone) { - int pos = pendingMessages.front(); - pendingMessages.pop_front(); - - guard.unlock(); - Process(pos); - guard.lock(); - } - if (pendingMessagesDone) { - break; - } - pendingMessageCond.wait(guard); - } - - return 0; - } - bool MessageAllowed() { if (!IsEnabled() || CheckSpamLimited()) return false; @@ -680,33 +598,20 @@ namespace Reporting } void SendReportMessage(const char *message, const char *formatted) { - int pos = NextFreePos(); - if (pos == -1) - return; + // MessageAllowed is checked first. - Payload &payload = payloadBuffer[pos]; + Payload payload{}; payload.type = RequestType::MESSAGE; payload.string1 = message; payload.string2 = formatted; - std::lock_guard guard(pendingMessageLock); - pendingMessages.push_back(pos); - pendingMessageCond.notify_one(); - - if (!messageThread.joinable()) { - messageThread = std::thread(SendPendingReportsThread); - } + Process(payload); } - void ReportCompatibility(const char *compat, int graphics, int speed, int gameplay, const std::string &screenshotFilename) - { + void ReportCompatibility(const char *compat, int graphics, int speed, int gameplay, const std::string &screenshotFilename) { if (!IsEnabled()) return; - int pos = NextFreePos(); - if (pos == -1) - return; - - Payload &payload = payloadBuffer[pos]; + Payload payload{}; payload.type = RequestType::COMPAT; payload.string1 = compat; payload.string2 = screenshotFilename; @@ -714,9 +619,7 @@ namespace Reporting payload.int2 = speed; payload.int3 = gameplay; - if (compatThread.joinable()) - compatThread.join(); - compatThread = std::thread(Process, pos); + Process(payload); } std::vector CompatibilitySuggestions() { diff --git a/UI/ReportScreen.cpp b/UI/ReportScreen.cpp index 6d65a67003..e698abf6d1 100644 --- a/UI/ReportScreen.cpp +++ b/UI/ReportScreen.cpp @@ -168,7 +168,7 @@ ReportScreen::ReportScreen(const Path &gamePath) // unused gamePath, after remo ScreenRenderFlags ReportScreen::render(ScreenRenderMode mode) { _dbg_assert_(mode & ScreenRenderMode::FIRST); - _dbg_assert_(mode & ScreenRenderMode::TOP); + // _dbg_assert_(mode & ScreenRenderMode::TOP); if (mode & ScreenRenderMode::TOP) { // We do this after render because we need it to be within the frame (so the screenshot works).