Skip to content

Commit f5409cb

Browse files
committed
Fix crashes when freeing models that are still in use. Addendum to 359f557
Main fix: - Don't delete model data while GTA still references it - Add checks in AddColRef/RemoveColRef to avoid crash on freed models Cleanup fixes: - Clear all cached default values when model is freed (was leaking to reused IDs) - Reset wrapper state so it's clean when reused for another model - Clear custom collision/clump pointers when blocking to avoid dangling refs Building fix: - SetModel now skips work if model didn't change - Destroy before Create to properly release old refs first Cons: blocked models leak until restart, but thats better than crashing. Added report log for when this happens.
1 parent 748adfa commit f5409cb

File tree

2 files changed

+66
-15
lines changed

2 files changed

+66
-15
lines changed

Client/game_sa/CModelInfoSA.cpp

Lines changed: 59 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1948,7 +1948,11 @@ void CModelInfoSA::AddColRef()
19481948
}
19491949
else
19501950
{
1951-
originalColModel = GetInterface()->pColModel;
1951+
CBaseModelInfoSAInterface* pInterface = GetInterface();
1952+
if (pInterface)
1953+
originalColModel = pInterface->pColModel;
1954+
else
1955+
AddReportLog(5552, SString("AddColRef called with null interface for model %u", m_dwModelID), 10);
19521956
}
19531957

19541958
if (originalColModel)
@@ -1969,7 +1973,11 @@ void CModelInfoSA::RemoveColRef()
19691973
}
19701974
else
19711975
{
1972-
originalColModel = GetInterface()->pColModel;
1976+
CBaseModelInfoSAInterface* pInterface = GetInterface();
1977+
if (pInterface)
1978+
originalColModel = pInterface->pColModel;
1979+
else
1980+
AddReportLog(5553, SString("RemoveColRef called with null interface for model %u", m_dwModelID), 10);
19731981
}
19741982

19751983
if (originalColModel)
@@ -2127,23 +2135,42 @@ void CModelInfoSA::MakeVehicleAutomobile(ushort usBaseID)
21272135

21282136
void CModelInfoSA::DeallocateModel(void)
21292137
{
2130-
// Model IDs can be reused (engineRequestModel); do not let a previous model's stored default TXD
2131-
// mapping leak into a later model that reuses the same ID.
2132-
ms_DefaultTxdIDMap.erase(static_cast<unsigned short>(m_dwModelID));
2133-
21342138
CBaseModelInfoSAInterface* pInterfaceToDelete = ppModelInfo[m_dwModelID];
21352139

21362140
if (!pInterfaceToDelete)
21372141
return;
21382142

2143+
// GTA's destructors (e.g. CObject at 0x4C4BB0) access ppModelInfo[] during cleanup.
2144+
// Block deletion while refs > 0 to avoid null pointer crash.
21392145
if (pInterfaceToDelete->usNumberOfRefs > 0)
21402146
{
2141-
g_pCore->LogEvent(550, "Model deallocation", "",
2142-
SString("Blocked DeallocateModel for model %d with %d active references",
2143-
m_dwModelID, pInterfaceToDelete->usNumberOfRefs), 5550);
2147+
AddReportLog(5550, SString("Blocked DeallocateModel for model %u with %u active refs to prevent crash at 0x4C4BB0",
2148+
m_dwModelID, static_cast<unsigned int>(pInterfaceToDelete->usNumberOfRefs)));
2149+
2150+
m_pInterface = pInterfaceToDelete;
2151+
2152+
// Clear custom model pointers to prevent use-after-free on later RemoveRef calls
2153+
m_pCustomClump = nullptr;
2154+
m_pCustomColModel = nullptr;
2155+
m_pOriginalColModelInterface = nullptr;
2156+
m_originalFlags = 0;
2157+
2158+
// Keep m_dwReferences and TXD mapping intact - model still in use
2159+
// Tradeoff: interface leaks until refs hit 0, model ID stays occupied
21442160
return;
21452161
}
21462162

2163+
// Clear stored defaults so they don't leak to a model that reuses this ID
2164+
ms_DefaultTxdIDMap.erase(static_cast<unsigned short>(m_dwModelID));
2165+
ms_ModelDefaultFlagsMap.erase(m_dwModelID);
2166+
ms_ModelDefaultLodDistanceMap.erase(m_dwModelID);
2167+
ms_ModelDefaultAlphaTransparencyMap.erase(m_dwModelID);
2168+
ms_OriginalObjectPropertiesGroups.erase(m_dwModelID);
2169+
ms_ModelDefaultDummiesPosition.erase(m_dwModelID);
2170+
ms_VehicleModelDefaultWheelSizes.erase(m_dwModelID);
2171+
2172+
pGame->GetStreaming()->RemoveModel(m_dwModelID);
2173+
21472174
// Capture model type and damageability BEFORE nulling the array entry.
21482175
// pInterfaceToDelete remains valid (it's a local pointer to the heap object),
21492176
// but we extract this info now to avoid any issues if vtable access were affected.
@@ -2155,12 +2182,32 @@ void CModelInfoSA::DeallocateModel(void)
21552182
isDamageableAtomic = (asDamageable != nullptr);
21562183
}
21572184

2158-
// Force streaming system to unload the model
2159-
pGame->GetStreaming()->RemoveModel(m_dwModelID);
2185+
// TIME model map uses interface pointer as key - must clean up before delete
2186+
if (modelType == eModelInfoType::TIME)
2187+
{
2188+
CTimeInfoSAInterface* pTimeInfo = &static_cast<CTimeModelInfoSAInterface*>(pInterfaceToDelete)->timeInfo;
2189+
auto it = ms_ModelDefaultModelTimeInfo.find(pTimeInfo);
2190+
if (it != ms_ModelDefaultModelTimeInfo.end())
2191+
{
2192+
delete it->second;
2193+
ms_ModelDefaultModelTimeInfo.erase(it);
2194+
}
2195+
}
21602196

21612197
// Null the array entry BEFORE delete for fail-fast if anything tries to access it during deletion
21622198
ppModelInfo[m_dwModelID] = nullptr;
21632199

2200+
// Reset wrapper state - this object persists and may be reused for a new model
2201+
m_pInterface = nullptr;
2202+
m_dwReferences = 0;
2203+
m_dwPendingInterfaceRef = 0;
2204+
m_dwParentID = 0;
2205+
m_pCustomClump = nullptr;
2206+
m_pCustomColModel = nullptr;
2207+
m_pOriginalColModelInterface = nullptr;
2208+
m_originalFlags = 0;
2209+
m_ModelSupportedUpgrades.Reset();
2210+
21642211
switch (modelType)
21652212
{
21662213
case eModelInfoType::VEHICLE:
@@ -2188,7 +2235,7 @@ void CModelInfoSA::DeallocateModel(void)
21882235
delete reinterpret_cast<CTimeModelInfoSAInterface*>(pInterfaceToDelete);
21892236
break;
21902237
default:
2191-
AddReportLog(5551, SString("Unknown model type %d for model %d - memory leaked to prevent corruption",
2238+
AddReportLog(5551, SString("Unknown model type %d for model %u - memory leaked to prevent corruption",
21922239
static_cast<int>(modelType), m_dwModelID));
21932240
return;
21942241
}

Client/mods/deathmatch/logic/CClientBuilding.cpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,13 @@ void CClientBuilding::SetModel(uint16_t model)
101101
{
102102
if (CClientBuildingManager::IsValidModel(model))
103103
{
104-
m_usModelId = model;
105-
m_pModelInfo = g_pGame->GetModelInfo(model);
106-
Recreate();
104+
if (model != m_usModelId)
105+
{
106+
Destroy();
107+
m_usModelId = model;
108+
m_pModelInfo = g_pGame->GetModelInfo(model);
109+
Create();
110+
}
107111
}
108112
}
109113

0 commit comments

Comments
 (0)