Addendum to 36ec08c (CEF rework #5) for some more potential mem leaks

Dutchman101 · Dutchman101 · commit 35303176a247 · 2025-11-22T23:18:51.000+08:00
diff --git a/Client/cefweb/CAjaxResourceHandler.cpp b/Client/cefweb/CAjaxResourceHandler.cpp
@@ -36,6 +36,12 @@ void CAjaxResourceHandler::SetResponse(std::string data)
     m_strResponse = std::move(data);
     m_bHasData = true;
 
+    // Clear request data to free memory as it's no longer needed
+    m_vecGetData.clear();
+    m_vecGetData.shrink_to_fit();
+    m_vecPostData.clear();
+    m_vecPostData.shrink_to_fit();
+
     if (!m_callback)
         return;
 
diff --git a/Client/cefweb/CWebCore.cpp b/Client/cefweb/CWebCore.cpp
@@ -27,6 +27,14 @@
     #pragma comment(lib, "cef_sandbox.lib")
 #endif
 
+CWebCore::EventEntry::EventEntry(const std::function<void()>& callback_, CWebView* pWebView_) : callback(callback_), pWebView(pWebView_) {}
+
+#ifdef MTA_DEBUG
+CWebCore::EventEntry::EventEntry(const std::function<void()>& callback_, CWebView* pWebView_, const SString& name_) : callback(callback_), pWebView(pWebView_), name(name_) {}
+#endif
+
+CWebCore::TaskEntry::TaskEntry(std::function<void(bool)> callback, CWebView* webView) : task(callback), webView(webView) {}
+
 CWebCore::CWebCore()
 {
     m_pRequestsGUI = nullptr;
@@ -363,7 +371,7 @@ void CWebCore::AddEventToEventQueue(std::function<void()> event, CWebView* pWebV
     if (pWebView && pWebView->IsBeingDestroyed())
         return;
 
-    std::lock_guard<std::mutex> lock(m_EventQueueMutex);
+    std::scoped_lock lock(m_EventQueueMutex);
 
     // Prevent unbounded queue growth - drop oldest events if queue is too large
     if (m_EventQueue.size() >= MAX_EVENT_QUEUE_SIZE)
@@ -386,7 +394,7 @@ void CWebCore::AddEventToEventQueue(std::function<void()> event, CWebView* pWebV
 
 void CWebCore::RemoveWebViewEvents(CWebView* pWebView)
 {
-    std::lock_guard<std::mutex> lock(m_EventQueueMutex);
+    std::scoped_lock lock(m_EventQueueMutex);
 
     for (auto iter = m_EventQueue.begin(); iter != m_EventQueue.end();)
     {
@@ -401,12 +409,16 @@ void CWebCore::DoEventQueuePulse()
 {
     std::list<EventEntry> eventQueue;
     {
-        std::lock_guard<std::mutex> lock(m_EventQueueMutex);
+        std::scoped_lock lock(m_EventQueueMutex);
         std::swap(eventQueue, m_EventQueue);
     }
 
     for (auto& event : eventQueue)
     {
+        // Skip event if the associated WebView is being destroyed
+        if (event.pWebView && event.pWebView->IsBeingDestroyed())
+            continue;
+
         event.callback();
     }
 
@@ -473,13 +485,20 @@ void CWebCore::DoTaskQueuePulse()
 
     for (TaskEntry& entry : taskQueue)
     {
+        // Abort task if the associated WebView is being destroyed
+        if (entry.webView && entry.webView->IsBeingDestroyed())
+        {
+            entry.task(true);
+            continue;
+        }
+
         entry.task(false);
     }
 }
 
 eURLState CWebCore::GetDomainState(const SString& strURL, bool bOutputDebug)
 {
-    std::lock_guard<std::recursive_mutex> lock(m_FilterMutex);
+    std::scoped_lock lock(m_FilterMutex);
 
     // Initialize wildcard whitelist (be careful with modifying) | Todo: Think about the following
     static constexpr const char* wildcardWhitelist[] = {"*.googlevideo.com", "*.google.com",  "*.youtube.com",    "*.ytimg.com",
@@ -491,8 +510,7 @@ eURLState CWebCore::GetDomainState(const SString& strURL, bool bOutputDebug)
             return eURLState::WEBPAGE_ALLOWED;
     }
 
-    google::dense_hash_map<SString, WebFilterPair>::iterator iter = m_Whitelist.find(strURL);
-    if (iter != m_Whitelist.end())
+    if (auto iter = m_Whitelist.find(strURL); iter != m_Whitelist.end())
     {
         if (iter->second.first == true)
             return eURLState::WEBPAGE_ALLOWED;
@@ -583,7 +601,7 @@ void CWebCore::InitialiseWhiteAndBlacklist(bool bAddHardcoded, bool bAddDynamic)
 
 void CWebCore::AddAllowedPage(const SString& strURL, eWebFilterType filterType)
 {
-    std::lock_guard<std::recursive_mutex> lock(m_FilterMutex);
+    std::scoped_lock lock(m_FilterMutex);
 
     // Prevent unbounded whitelist growth - remove old REQUEST entries if limit reached
     if (m_Whitelist.size() >= 50000)
@@ -603,7 +621,7 @@ void CWebCore::AddAllowedPage(const SString& strURL, eWebFilterType filterType)
 
 void CWebCore::AddBlockedPage(const SString& strURL, eWebFilterType filterType)
 {
-    std::lock_guard<std::recursive_mutex> lock(m_FilterMutex);
+    std::scoped_lock lock(m_FilterMutex);
 
     // Prevent unbounded whitelist growth - remove old REQUEST entries if limit reached
     if (m_Whitelist.size() >= 50000)
@@ -623,6 +641,13 @@ void CWebCore::AddBlockedPage(const SString& strURL, eWebFilterType filterType)
 
 void CWebCore::RequestPages(const std::vector<SString>& pages, WebRequestCallback* pCallback)
 {
+    if (m_PendingRequests.size() >= MAX_PENDING_REQUESTS)
+    {
+        if (pCallback)
+            (*pCallback)(false, std::unordered_set<SString>(pages.begin(), pages.end()));
+        return;
+    }
+
     // Add to pending pages queue
     bool bNewItem = false;
     for (const auto& page : pages)
@@ -631,8 +656,9 @@ void CWebCore::RequestPages(const std::vector<SString>& pages, WebRequestCallbac
         if (status == eURLState::WEBPAGE_ALLOWED || status == eURLState::WEBPAGE_DISALLOWED)
             continue;
 
-        m_PendingRequests.insert(page);
-        bNewItem = true;
+        const auto [iter, inserted] = m_PendingRequests.insert(page);
+        if (inserted)
+            bNewItem = true;
     }
 
     if (bNewItem)
diff --git a/Client/cefweb/CWebCore.h b/Client/cefweb/CWebCore.h
@@ -23,6 +23,7 @@
 #define MAX_EVENT_QUEUE_SIZE 10000
 #define MAX_TASK_QUEUE_SIZE 1000
 #define MAX_WHITELIST_SIZE 50000
+#define MAX_PENDING_REQUESTS 100
 #define GetNextSibling(hwnd) GetWindow(hwnd, GW_HWNDNEXT) // Re-define the conflicting macro
 #define GetFirstChild(hwnd) GetTopWindow(hwnd)
 
@@ -36,23 +37,23 @@ class CWebCore : public CWebCoreInterface
     struct EventEntry
     {
         std::function<void()> callback;
-        CWebView*             pWebView;
+        CefRefPtr<CWebView>   pWebView;
     #ifdef MTA_DEBUG
         SString name;
     #endif
 
-        EventEntry(const std::function<void()>& callback_, CWebView* pWebView_) : callback(callback_), pWebView(pWebView_) {}
+        EventEntry(const std::function<void()>& callback_, CWebView* pWebView_);
 #ifdef MTA_DEBUG
-        EventEntry(const std::function<void()>& callback_, CWebView* pWebView_, const SString& name_) : callback(callback_), pWebView(pWebView_), name(name_) {}
+        EventEntry(const std::function<void()>& callback_, CWebView* pWebView_, const SString& name_);
 #endif
     };
 
     struct TaskEntry
     {
         std::packaged_task<void(bool)> task;
-        CWebView*                      webView;
+        CefRefPtr<CWebView>            webView;
 
-        TaskEntry(std::function<void(bool)> callback, CWebView* webView) : task(callback), webView(webView) {}
+        TaskEntry(std::function<void(bool)> callback, CWebView* webView);
     };
 
 public:
diff --git a/Client/cefweb/CWebView.cpp b/Client/cefweb/CWebView.cpp
@@ -25,6 +25,9 @@ CWebView::CWebView(bool bIsLocal, CWebBrowserItem* pWebBrowserRenderItem, bool b
     m_bIsLocal = bIsLocal;
     m_bIsTransparent = bTransparent;
     m_pWebBrowserRenderItem = pWebBrowserRenderItem;
+    if (m_pWebBrowserRenderItem)
+        m_pWebBrowserRenderItem->AddRef();
+
     m_pEventsInterface = nullptr;
     m_bBeingDestroyed = false;
     m_fVolume = 1.0f;
@@ -47,6 +50,12 @@ CWebView::~CWebView()
         }
     }
 
+    if (m_pWebBrowserRenderItem)
+    {
+        m_pWebBrowserRenderItem->Release();
+        m_pWebBrowserRenderItem = nullptr;
+    }
+
     // Make sure we don't dead lock the CEF render thread
     ResumeCefThread();
 
@@ -280,7 +289,7 @@ void CWebView::ClearTexture()
 
 void CWebView::UpdateTexture()
 {
-    const std::lock_guard lock(m_RenderData.dataMutex);
+    const std::scoped_lock lock(m_RenderData.dataMutex);
 
     // Validate render item exists before accessing
     if (!m_pWebBrowserRenderItem) [[unlikely]]
@@ -714,23 +723,23 @@ void CWebView::GetSourceCode(const std::function<void(const std::string& code)>&
     class MyStringVisitor : public CefStringVisitor
     {
     private:
-        CWebView*                               webView;
+        CefRefPtr<CWebView>                     webView;
         std::function<void(const std::string&)> callback;
 
     public:
         MyStringVisitor(CWebView* webView_, const std::function<void(const std::string&)>& callback_) : webView(webView_), callback(callback_) {}
 
         virtual void Visit(const CefString& code) override
         {
-            // Avoid UAF
+            // Check if webview is being destroyed to prevent UAF
             if (webView->IsBeingDestroyed())
                 return;
             
             // Limit to 2MiB for now to prevent freezes (TODO: Optimize that and increase later)
             if (code.size() <= 2097152)
             {
                 // Call callback on main thread
-                g_pCore->GetWebCore()->AddEventToEventQueue(std::bind(callback, code), webView, "GetSourceCode_Visit");
+                g_pCore->GetWebCore()->AddEventToEventQueue(std::bind(callback, code), webView.get(), "GetSourceCode_Visit");
             }
         }
 
@@ -783,8 +792,8 @@ bool CWebView::GetFullPathFromLocal(SString& strPath)
 
 bool CWebView::RegisterAjaxHandler(const SString& strURL)
 {
-    auto result = m_AjaxHandlers.insert(strURL);
-    return result.second;
+    auto [iter, inserted] = m_AjaxHandlers.insert(strURL);
+    return inserted;
 }
 
 bool CWebView::UnregisterAjaxHandler(const SString& strURL)
diff --git a/Client/cefweb/CWebsiteRequests.cpp b/Client/cefweb/CWebsiteRequests.cpp
@@ -63,13 +63,8 @@ CWebsiteRequests::CWebsiteRequests()
 
 CWebsiteRequests::~CWebsiteRequests()
 {
-    delete m_pLabel1;
-    delete m_pLabel2;
-    delete m_pAddressMemo;
-    delete m_pCheckRemember;
-    delete m_pButtonAllow;
-    delete m_pButtonDeny;
-    delete m_pWindow;
+    if (m_pWindow)
+        g_pCore->GetGUI()->DestroyElementRecursive(m_pWindow);
 }
 
 void CWebsiteRequests::Show()
@@ -126,8 +121,11 @@ void CWebsiteRequests::Callback(bool bAllow, const std::unordered_set<SString>&
 bool CWebsiteRequests::OnAllowButtonClick(CGUIElement* pElement)
 {
     Hide();
-    const auto pWebCore = g_pCore->GetWebCore();
-    const auto requests = pWebCore ? pWebCore->AllowPendingPages(m_pCheckRemember->GetSelected()) : std::unordered_set<SString>();
+    std::unordered_set<SString> requests;
+    if (auto pWebCore = g_pCore->GetWebCore(); pWebCore)
+    {
+        requests = pWebCore->AllowPendingPages(m_pCheckRemember->GetSelected());
+    }
     Callback(true, requests);
 
     return true;
@@ -136,8 +134,11 @@ bool CWebsiteRequests::OnAllowButtonClick(CGUIElement* pElement)
 bool CWebsiteRequests::OnDenyButtonClick(CGUIElement* pElement)
 {
     Hide();
-    const auto pWebCore = g_pCore->GetWebCore();
-    const auto requests = pWebCore ? pWebCore->DenyPendingPages() : std::unordered_set<SString>();
+    std::unordered_set<SString> requests;
+    if (auto pWebCore = g_pCore->GetWebCore(); pWebCore)
+    {
+        requests = pWebCore->DenyPendingPages();
+    }
     Callback(false, requests);
 
     return true;
diff --git a/Client/sdk/core/CRenderItemManagerInterface.h b/Client/sdk/core/CRenderItemManagerInterface.h
@@ -9,6 +9,8 @@
  *
  *****************************************************************************/
 
+#pragma once
+
 #include <CVector.h>
 #include <CVector2D.h>
 

Original file line number	Diff line number	Diff line change
`@@ -9,6 +9,8 @@`
`9`	`9`	`*`
`10`	`10`	`*****************************************************************************/`
`11`	`11`
	`12`	`+#pragma once`
	`13`	`+`
`12`	`14`	`#include <CVector.h>`
`13`	`15`	`#include <CVector2D.h>`
`14`	`16`