Skip to content

Commit 1a732bf

Browse files
Synchronize changes from 1.6 branch [ci skip]
c27d6b3 Fix SA crash at 0x002a65ef f17490e CEF memory leaks #2 bd7a2af CEF memory leaks #1
2 parents e534426 + c27d6b3 commit 1a732bf

File tree

9 files changed

+146
-6
lines changed

9 files changed

+146
-6
lines changed

Client/cefweb/CAjaxResourceHandler.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,15 @@ CAjaxResourceHandler::CAjaxResourceHandler(std::vector<SString>& vecGet, std::ve
1818
{
1919
}
2020

21+
CAjaxResourceHandler::~CAjaxResourceHandler()
22+
{
23+
// Ensure callback is released if handler is destroyed before completion
24+
if (m_callback)
25+
{
26+
m_callback = nullptr;
27+
}
28+
}
29+
2130
std::vector<SString>& CAjaxResourceHandler::GetGetData()
2231
{
2332
return m_vecGetData;
@@ -34,12 +43,18 @@ void CAjaxResourceHandler::SetResponse(const SString& data)
3443
m_bHasData = true;
3544

3645
if (m_callback)
46+
{
3747
m_callback->Continue();
48+
// Release callback to prevent memory leak
49+
m_callback = nullptr;
50+
}
3851
}
3952

4053
// CefResourceHandler implementation
4154
void CAjaxResourceHandler::Cancel()
4255
{
56+
// Release callback reference on cancellation to prevent memory leak
57+
m_callback = nullptr;
4358
}
4459

4560
void CAjaxResourceHandler::GetResponseHeaders(CefRefPtr<CefResponse> response, int64& response_length, CefString& redirectUrl)

Client/cefweb/CAjaxResourceHandler.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ class CAjaxResourceHandler : public CefResourceHandler, public CAjaxResourceHand
2020
{
2121
public:
2222
CAjaxResourceHandler(std::vector<SString>& vecGet, std::vector<SString>& vecPost, const CefString& mimeType);
23+
virtual ~CAjaxResourceHandler();
2324

2425
virtual std::vector<SString>& GetGetData() override;
2526
virtual std::vector<SString>& GetPostData() override;

Client/cefweb/CWebCore.cpp

Lines changed: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,13 +109,22 @@ void CWebCore::DestroyWebView(CWebViewInterface* pWebViewInterface)
109109
CefRefPtr<CWebView> pWebView = dynamic_cast<CWebView*>(pWebViewInterface);
110110
if (pWebView)
111111
{
112+
// Mark as being destroyed to prevent new events/tasks
113+
pWebView->SetBeingDestroyed(true);
114+
112115
// Ensure that no attached events or tasks are in the queue
113116
RemoveWebViewEvents(pWebView.get());
114117
RemoveWebViewTasks(pWebView.get());
115118

119+
// Remove from list before closing to break reference cycles early
116120
m_WebViews.remove(pWebView);
117-
// pWebView->Release(); // Do not release since other references get corrupted then
121+
122+
// CloseBrowser will eventually trigger OnBeforeClose which clears m_pWebView
123+
// This breaks the circular reference: CWebView -> CefBrowser -> CWebView
118124
pWebView->CloseBrowser();
125+
126+
// Note: Do not call Release() - let CefRefPtr manage the lifecycle
127+
// The circular reference is broken via OnBeforeClose setting m_pWebView = nullptr
119128
}
120129
}
121130

@@ -164,6 +173,18 @@ void CWebCore::AddEventToEventQueue(std::function<void()> event, CWebView* pWebV
164173

165174
std::lock_guard<std::mutex> lock(m_EventQueueMutex);
166175

176+
// Prevent unbounded queue growth - drop oldest events if queue is too large
177+
if (m_EventQueue.size() >= MAX_EVENT_QUEUE_SIZE)
178+
{
179+
// Log warning even in release builds as this indicates a serious issue
180+
g_pCore->GetConsole()->Printf("WARNING: Browser event queue size limit reached (%d), dropping oldest events", MAX_EVENT_QUEUE_SIZE);
181+
182+
// Remove oldest 10% of events to make room
183+
auto removeCount = MAX_EVENT_QUEUE_SIZE / 10;
184+
for (size_t i = 0; i < removeCount && !m_EventQueue.empty(); ++i)
185+
m_EventQueue.pop_front();
186+
}
187+
167188
#ifndef MTA_DEBUG
168189
m_EventQueue.push_back(EventEntry(event, pWebView));
169190
#else
@@ -215,6 +236,22 @@ void CWebCore::WaitForTask(std::function<void(bool)> task, CWebView* webView)
215236
std::future<void> result;
216237
{
217238
std::scoped_lock lock(m_TaskQueueMutex);
239+
240+
// Prevent unbounded queue growth - if queue is too large, oldest tasks will be dropped during pulse
241+
if (m_TaskQueue.size() >= MAX_TASK_QUEUE_SIZE)
242+
{
243+
#ifdef MTA_DEBUG
244+
g_pCore->GetConsole()->Printf("Warning: Task queue size limit reached (%d), task will be aborted", MAX_TASK_QUEUE_SIZE);
245+
#endif
246+
// Must still queue the task to fulfill the future, but it will be aborted during processing
247+
// Removing oldest task to make room
248+
if (!m_TaskQueue.empty())
249+
{
250+
m_TaskQueue.front().task(true); // Abort oldest task
251+
m_TaskQueue.pop_front();
252+
}
253+
}
254+
218255
m_TaskQueue.emplace_back(TaskEntry{task, webView});
219256
result = m_TaskQueue.back().task.get_future();
220257
}
@@ -358,13 +395,39 @@ void CWebCore::AddAllowedPage(const SString& strURL, eWebFilterType filterType)
358395
{
359396
std::lock_guard<std::recursive_mutex> lock(m_FilterMutex);
360397

398+
// Prevent unbounded whitelist growth - remove old REQUEST entries if limit reached
399+
if (m_Whitelist.size() >= MAX_WHITELIST_SIZE)
400+
{
401+
// Remove WEBFILTER_REQUEST entries (temporary session entries)
402+
for (auto iter = m_Whitelist.begin(); iter != m_Whitelist.end();)
403+
{
404+
if (iter->second.second == eWebFilterType::WEBFILTER_REQUEST)
405+
m_Whitelist.erase(iter++);
406+
else
407+
++iter;
408+
}
409+
}
410+
361411
m_Whitelist[strURL] = std::pair<bool, eWebFilterType>(true, filterType);
362412
}
363413

364414
void CWebCore::AddBlockedPage(const SString& strURL, eWebFilterType filterType)
365415
{
366416
std::lock_guard<std::recursive_mutex> lock(m_FilterMutex);
367417

418+
// Prevent unbounded whitelist growth - remove old REQUEST entries if limit reached
419+
if (m_Whitelist.size() >= MAX_WHITELIST_SIZE)
420+
{
421+
// Remove WEBFILTER_REQUEST entries (temporary session entries)
422+
for (auto iter = m_Whitelist.begin(); iter != m_Whitelist.end();)
423+
{
424+
if (iter->second.second == eWebFilterType::WEBFILTER_REQUEST)
425+
m_Whitelist.erase(iter++);
426+
else
427+
++iter;
428+
}
429+
}
430+
368431
m_Whitelist[strURL] = std::pair<bool, eWebFilterType>(false, filterType);
369432
}
370433

Client/cefweb/CWebCore.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@
2020
#define MTA_BROWSERDATA_PATH "mta/cef/browserdata.xml"
2121
#define BROWSER_LIST_UPDATE_INTERVAL (24*60*60)
2222
#define BROWSER_UPDATE_URL "https://cef.multitheftauto.com/get.php"
23+
#define MAX_EVENT_QUEUE_SIZE 10000
24+
#define MAX_TASK_QUEUE_SIZE 1000
25+
#define MAX_WHITELIST_SIZE 50000
2326
#define GetNextSibling(hwnd) GetWindow(hwnd, GW_HWNDNEXT) // Re-define the conflicting macro
2427
#define GetFirstChild(hwnd) GetTopWindow(hwnd)
2528

Client/cefweb/CWebView.cpp

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,15 @@ CWebView::~CWebView()
4444
// Make sure we don't dead lock the CEF render thread
4545
ResumeCefThread();
4646

47-
// Ensure that CefRefPtr::~CefRefPtr doesn't try to release it twice (it has already been released in CWebView::OnBeforeClose)
48-
m_pWebView = nullptr;
47+
// Clean up AJAX handlers to prevent accumulation
48+
m_AjaxHandlers.clear();
49+
50+
// Break circular reference: ensure browser reference is cleared
51+
// This is to prevent memory leaks from CWebView <-> CefBrowser cycles
52+
if (m_pWebView)
53+
{
54+
m_pWebView = nullptr;
55+
}
4956

5057
OutputDebugLine("CWebView::~CWebView");
5158
}
@@ -83,6 +90,9 @@ void CWebView::CloseBrowser()
8390
// Make sure we don't dead lock the CEF render thread
8491
ResumeCefThread();
8592

93+
// Clear AJAX handlers early to prevent late event processing
94+
m_AjaxHandlers.clear();
95+
8696
if (m_pWebView)
8797
m_pWebView->GetHost()->CloseBrowser(true);
8898
}
@@ -500,8 +510,12 @@ bool CWebView::HasAjaxHandler(const SString& strURL)
500510

501511
void CWebView::HandleAjaxRequest(const SString& strURL, CAjaxResourceHandler* pHandler)
502512
{
503-
auto func = std::bind(&CWebBrowserEventsInterface::Events_OnAjaxRequest, m_pEventsInterface, pHandler, strURL);
504-
g_pCore->GetWebCore()->AddEventToEventQueue(func, this, "AjaxResourceRequest");
513+
// Only queue event if not being destroyed to prevent UAF
514+
if (!m_bBeingDestroyed)
515+
{
516+
auto func = std::bind(&CWebBrowserEventsInterface::Events_OnAjaxRequest, m_pEventsInterface, pHandler, strURL);
517+
g_pCore->GetWebCore()->AddEventToEventQueue(func, this, "AjaxResourceRequest");
518+
}
505519
}
506520

507521
bool CWebView::ToggleDevTools(bool visible)
@@ -719,6 +733,8 @@ void CWebView::OnPaint(CefRefPtr<CefBrowser> browser, CefRenderHandler::PaintEle
719733
m_RenderData.width = width;
720734
m_RenderData.height = height;
721735
m_RenderData.dirtyRects = dirtyRects;
736+
// Prevent vector capacity growth memory leak - shrink excess capacity
737+
m_RenderData.dirtyRects.shrink_to_fit();
722738
m_RenderData.changed = true;
723739

724740
// Wait for the main thread to handle drawing the texture

Client/game_sa/CVehicleSA.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1809,7 +1809,7 @@ void CVehicleSA::RecalculateSuspensionLines()
18091809

18101810
DWORD dwModel = GetModelIndex();
18111811
CModelInfo* pModelInfo = pGame->GetModelInfo(dwModel);
1812-
if (pModelInfo && pModelInfo->IsMonsterTruck() || pModelInfo->IsCar())
1812+
if (pModelInfo && (pModelInfo->IsCar() || pModelInfo->IsMonsterTruck() || pModelInfo->IsTrailer()))
18131813
{
18141814
// Trains (Their trailers do as well!)
18151815
if (pModelInfo->IsTrain() || dwModel == 571 || dwModel == 570 || dwModel == 569 || dwModel == 590)

Client/mods/deathmatch/logic/CClientVehicle.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4300,6 +4300,11 @@ void CClientVehicle::ApplyHandling()
43004300
if (!m_pVehicle)
43014301
return;
43024302

4303+
// Ensure model is loaded before recalculating handling
4304+
CModelInfo* pModelInfo = g_pGame->GetModelInfo(GetModel());
4305+
if (!pModelInfo || !pModelInfo->IsLoaded())
4306+
return;
4307+
43034308
m_pVehicle->RecalculateHandling();
43044309

43054310
if (m_eVehicleType == CLIENTVEHICLE_BMX || m_eVehicleType == CLIENTVEHICLE_BIKE)

Client/mods/deathmatch/logic/CStaticFunctionDefinitions.cpp

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8905,6 +8905,10 @@ bool CStaticFunctionDefinitions::SetVehicleHandling(CClientVehicle* pVehicle, Ha
89058905
{
89068906
if (SetEntryHandling(pEntry, eProperty, ucValue))
89078907
{
8908+
CModelInfo* pModelInfo = g_pGame->GetModelInfo(pVehicle->GetModel());
8909+
if (!pModelInfo || !pModelInfo->IsLoaded())
8910+
return false;
8911+
89088912
pVehicle->ApplyHandling();
89098913
return true;
89108914
}
@@ -8922,6 +8926,10 @@ bool CStaticFunctionDefinitions::SetVehicleHandling(CClientVehicle* pVehicle, Ha
89228926
{
89238927
if (SetEntryHandling(pEntry, eProperty, uiValue))
89248928
{
8929+
CModelInfo* pModelInfo = g_pGame->GetModelInfo(pVehicle->GetModel());
8930+
if (!pModelInfo || !pModelInfo->IsLoaded())
8931+
return false;
8932+
89258933
pVehicle->ApplyHandling();
89268934
return true;
89278935
}
@@ -8939,6 +8947,10 @@ bool CStaticFunctionDefinitions::SetVehicleHandling(CClientVehicle* pVehicle, Ha
89398947
{
89408948
if (SetEntryHandling(pEntry, eProperty, fValue))
89418949
{
8950+
CModelInfo* pModelInfo = g_pGame->GetModelInfo(pVehicle->GetModel());
8951+
if (!pModelInfo || !pModelInfo->IsLoaded())
8952+
return false;
8953+
89428954
pVehicle->ApplyHandling();
89438955
return true;
89448956
}
@@ -8956,6 +8968,10 @@ bool CStaticFunctionDefinitions::SetVehicleHandling(CClientVehicle* pVehicle, Ha
89568968
{
89578969
if (SetEntryHandling(pEntry, eProperty, strValue))
89588970
{
8971+
CModelInfo* pModelInfo = g_pGame->GetModelInfo(pVehicle->GetModel());
8972+
if (!pModelInfo || !pModelInfo->IsLoaded())
8973+
return false;
8974+
89598975
pVehicle->ApplyHandling();
89608976
return true;
89618977
}
@@ -8973,6 +8989,10 @@ bool CStaticFunctionDefinitions::SetVehicleHandling(CClientVehicle* pVehicle, Ha
89738989
{
89748990
if (SetEntryHandling(pEntry, eProperty, vecValue))
89758991
{
8992+
CModelInfo* pModelInfo = g_pGame->GetModelInfo(pVehicle->GetModel());
8993+
if (!pModelInfo || !pModelInfo->IsLoaded())
8994+
return false;
8995+
89768996
pVehicle->ApplyHandling();
89778997
return true;
89788998
}
@@ -9031,6 +9051,10 @@ bool CStaticFunctionDefinitions::ResetVehicleHandling(CClientVehicle* pVehicle)
90319051
pEntry->SetSuspensionUpperLimit(pEntry->GetSuspensionLowerLimit() - 0.1f);
90329052
}
90339053

9054+
CModelInfo* pModelInfo = g_pGame->GetModelInfo(pVehicle->GetModel());
9055+
if (!pModelInfo || !pModelInfo->IsLoaded())
9056+
return false;
9057+
90349058
pVehicle->ApplyHandling();
90359059

90369060
return true;
@@ -9080,6 +9104,10 @@ bool CStaticFunctionDefinitions::ResetVehicleHandlingProperty(CClientVehicle* pV
90809104
return false;
90819105
}
90829106

9107+
CModelInfo* pModelInfo = g_pGame->GetModelInfo(pVehicle->GetModel());
9108+
if (!pModelInfo || !pModelInfo->IsLoaded())
9109+
return false;
9110+
90839111
pVehicle->ApplyHandling();
90849112

90859113
return true;

Client/mods/deathmatch/logic/luadefs/CLuaVehicleDefs.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2518,6 +2518,15 @@ int CLuaVehicleDefs::SetVehicleHandling(lua_State* luaVM)
25182518

25192519
if (!argStream.HasErrors())
25202520
{
2521+
// Check if the vehicle model is loaded
2522+
CModelInfo* pModelInfo = g_pGame->GetModelInfo(pVehicle->GetModel());
2523+
if (!pModelInfo || !pModelInfo->IsLoaded())
2524+
{
2525+
m_pScriptDebugging->LogWarning(luaVM, "setVehicleHandling failed: vehicle model not loaded");
2526+
lua_pushboolean(luaVM, false);
2527+
return 1;
2528+
}
2529+
25212530
if (argStream.NextIsString())
25222531
{
25232532
SString strProperty;

0 commit comments

Comments
 (0)