Skip to content

Commit 9b194aa

Browse files
Synchronize changes from 1.6 master branch [ci skip]
20a14e8 Addendum #2 to 49e5265 (after c787113), to include fixes for MTA's top 5 crashes that include SA offsets 0x003F3A17 & 0x003F374A, as well as other bugs and memory leaks. b18a784 Update client en_US pot
2 parents 26eab9f + 20a14e8 commit 9b194aa

File tree

3 files changed

+194
-49
lines changed

3 files changed

+194
-49
lines changed

Client/game_sa/CRenderWareSA.TextureReplacing.cpp

Lines changed: 135 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,11 @@ namespace
175175
m_owned = false;
176176
}
177177

178+
[[nodiscard]] bool IsOwned() const noexcept
179+
{
180+
return m_owned;
181+
}
182+
178183
~ScopedTxdRef()
179184
{
180185
if (m_owned)
@@ -646,7 +651,7 @@ namespace
646651

647652

648653
// AcquireModelTexturesInfo: Find/create texture info for a modelid
649-
ModelTexturesInfoGuard AcquireModelTexturesInfo(ushort usModelId)
654+
[[nodiscard]] ModelTexturesInfoGuard AcquireModelTexturesInfo(ushort usModelId)
650655
{
651656
ModelTexturesInfoGuard guard{std::unique_lock<std::recursive_mutex>(ms_ModelTexturesInfoMutex)};
652657
guard.modelInfo = dynamic_cast<CModelInfoSA*>(pGame->GetModelInfo(usModelId));
@@ -658,6 +663,21 @@ ModelTexturesInfoGuard AcquireModelTexturesInfo(ushort usModelId)
658663
if (auto infoIt = ms_ModelTexturesInfoMap.find(usTxdId); infoIt != ms_ModelTexturesInfoMap.end())
659664
{
660665
guard.info = &infoIt->second;
666+
667+
// Revalidate cached TXD pointer - it may have been unloaded by streaming system
668+
if (!IsValidTexDictionary(guard.info->pTxd))
669+
{
670+
// TXD was unloaded - try to reload it
671+
guard.info->pTxd = CTxdStore_GetTxd(usTxdId);
672+
673+
// If still invalid, the cached entry is stale and needs rebuild
674+
if (!IsValidTexDictionary(guard.info->pTxd))
675+
{
676+
// Invalidate the entry - caller should handle this
677+
guard.info->pTxd = nullptr;
678+
}
679+
}
680+
661681
return guard;
662682
}
663683

@@ -666,26 +686,45 @@ ModelTexturesInfoGuard AcquireModelTexturesInfo(ushort usModelId)
666686

667687
if (!pTxd)
668688
{
669-
guard.modelInfo->Request(BLOCKING, "CRenderWareSA::GetModelTexturesInfo");
670-
txdRefGuard.Acquire(usTxdId);
671-
((void(__cdecl*)(unsigned short))FUNC_RemoveModel)(usModelId);
672-
pTxd = CTxdStore_GetTxd(usTxdId);
673-
689+
// TXD not loaded - try to request it based on model type
690+
const eModelInfoType modelType = guard.modelInfo->GetModelType();
691+
692+
// For vehicles, we need the TXD loaded to prevent white textures
693+
// For weapons and other models, use lazy loading to avoid freezes with large texture packs
694+
if (modelType == eModelInfoType::VEHICLE)
695+
{
696+
// Request the model non-blocking first to avoid freezing
697+
guard.modelInfo->Request(NON_BLOCKING, "CRenderWareSA::AcquireModelTexturesInfo");
698+
pTxd = CTxdStore_GetTxd(usTxdId);
699+
700+
// If still not available, try blocking as last resort for vehicles
701+
if (!pTxd)
702+
{
703+
guard.modelInfo->Request(BLOCKING, "CRenderWareSA::AcquireModelTexturesInfo");
704+
// Remove model after blocking request to ensure proper reload
705+
((void(__cdecl*)(unsigned short))FUNC_RemoveModel)(usModelId);
706+
pTxd = CTxdStore_GetTxd(usTxdId);
707+
}
708+
}
709+
710+
// If still unavailable after attempts, fail gracefully
674711
if (!pTxd)
675712
return guard;
676713
}
677-
else
714+
715+
// Acquire TXD reference only after confirming TXD is available
716+
txdRefGuard.Acquire(usTxdId);
717+
718+
const eModelInfoType modelType = guard.modelInfo->GetModelType();
719+
if (modelType == eModelInfoType::PED)
678720
{
679-
txdRefGuard.Acquire(usTxdId);
680-
if (guard.modelInfo->GetModelType() == eModelInfoType::PED)
681-
{
682-
// Mystery fix for #9336: (MTA sometimes fails at loading custom textures)
683-
// Possibly forces the ped model to be reloaded in some way
684-
((void(__cdecl*)(unsigned short))FUNC_RemoveModel)(usModelId);
685-
}
721+
// Mystery fix for #9336: (MTA sometimes fails at loading custom textures)
722+
// Possibly forces the ped model to be reloaded in some way
723+
((void(__cdecl*)(unsigned short))FUNC_RemoveModel)(usModelId);
686724
}
687725

688-
auto [insertIt, inserted] = ms_ModelTexturesInfoMap.emplace(usTxdId, CModelTexturesInfo());
726+
auto result = ms_ModelTexturesInfoMap.emplace(usTxdId, CModelTexturesInfo());
727+
auto insertIt = result.first;
689728
guard.info = &insertIt->second;
690729

691730
guard.info->usTxdId = usTxdId;
@@ -694,13 +733,25 @@ ModelTexturesInfoGuard AcquireModelTexturesInfo(ushort usModelId)
694733
guard.info->originalTextures.clear();
695734
std::vector<RwTexture*> originals;
696735

697-
CRenderWareSA::GetTxdTextures(originals, guard.info->pTxd);
736+
// Revalidate pTxd before passing to GetTxdTextures to prevent crashes from stale pointers
737+
if (!IsValidTexDictionary(pTxd))
738+
{
739+
ms_ModelTexturesInfoMap.erase(insertIt);
740+
guard.info = nullptr;
741+
txdRefGuard.Release();
742+
CTxdStore_RemoveRef(usTxdId);
743+
return guard;
744+
}
745+
746+
CRenderWareSA::GetTxdTextures(originals, pTxd);
698747

699748
// If texture enumeration failed
700-
if (originals.empty())
749+
if (std::empty(originals))
701750
{
702751
ms_ModelTexturesInfoMap.erase(insertIt);
703752
guard.info = nullptr;
753+
txdRefGuard.Release();
754+
CTxdStore_RemoveRef(usTxdId);
704755
return guard;
705756
}
706757

@@ -727,35 +778,48 @@ ModelTexturesInfoGuard AcquireModelTexturesInfo(ushort usModelId)
727778
bool CRenderWareSA::ModelInfoTXDLoadTextures(SReplacementTextures* pReplacementTextures, const SString& strFilename, const SString& buffer,
728779
bool bFilteringEnabled)
729780
{
781+
// Validate input pointer first
782+
if (!pReplacementTextures)
783+
return false;
784+
730785
// Are we already loaded?
731-
if (!pReplacementTextures->textures.empty())
786+
if (!std::empty(pReplacementTextures->ownedTextures))
732787
return false;
733788

734789
// Try to load it
735790
RwTexDictionary* pTxd = ReadTXD(strFilename, buffer);
736791
if (pTxd)
737792
{
738-
// Get the list of textures into our own list
739-
GetTxdTextures(pReplacementTextures->textures, pTxd);
793+
// Get the list of textures into temporary vector
794+
std::vector<RwTexture*> loadedTextures;
795+
GetTxdTextures(loadedTextures, pTxd);
740796

741-
// Remove any null textures and configure valid ones
742-
auto newEnd = std::remove_if(pReplacementTextures->textures.begin(), pReplacementTextures->textures.end(),
743-
[](RwTexture* const pTex) { return pTex == nullptr; });
744-
pReplacementTextures->textures.erase(newEnd, pReplacementTextures->textures.end());
797+
// Make the txd forget it has any textures before destroying it
798+
pTxd->textures.root.next = &pTxd->textures.root;
799+
pTxd->textures.root.prev = &pTxd->textures.root;
800+
RwTexDictionaryDestroy(pTxd);
801+
pTxd = nullptr;
745802

746-
for (RwTexture* pTexture : pReplacementTextures->textures)
803+
// Transfer ownership to unique_ptr with custom deleter
804+
pReplacementTextures->ownedTextures.reserve(loadedTextures.size());
805+
pReplacementTextures->textures.reserve(loadedTextures.size());
806+
807+
for (RwTexture* pTexture : loadedTextures)
747808
{
809+
if (!pTexture)
810+
continue;
811+
812+
// Textures are now orphaned (txd was destroyed), we own them
748813
pTexture->txd = nullptr;
749814
if (bFilteringEnabled)
750815
pTexture->flags = kReplacementTextureFilterFlags;
816+
817+
// Take ownership with unique_ptr for automatic cleanup
818+
pReplacementTextures->ownedTextures.emplace_back(pTexture);
819+
// Keep raw pointer for compatibility with existing code
820+
pReplacementTextures->textures.push_back(pTexture);
751821
}
752822

753-
// Make the txd forget it has any textures and destroy it
754-
pTxd->textures.root.next = &pTxd->textures.root;
755-
pTxd->textures.root.prev = &pTxd->textures.root;
756-
RwTexDictionaryDestroy(pTxd);
757-
pTxd = nullptr;
758-
759823
// We succeeded if we got any textures
760824
return pReplacementTextures->textures.size() > 0;
761825
}
@@ -773,6 +837,10 @@ bool CRenderWareSA::ModelInfoTXDLoadTextures(SReplacementTextures* pReplacementT
773837
////////////////////////////////////////////////////////////////
774838
bool CRenderWareSA::ModelInfoTXDAddTextures(SReplacementTextures* pReplacementTextures, ushort usModelId)
775839
{
840+
// Validate input pointer first
841+
if (!pReplacementTextures)
842+
return false;
843+
776844
ModelTexturesInfoGuard infoGuard = AcquireModelTexturesInfo(usModelId);
777845
if (!infoGuard)
778846
return false;
@@ -793,6 +861,27 @@ bool CRenderWareSA::ModelInfoTXDAddTextures(SReplacementTextures* pReplacementTe
793861
if (!RegisterModelUsage(*pReplacementTextures, usModelId))
794862
return false;
795863

864+
// Prevent unbounded growth from spammy texture replacements on different TXD IDs (like at client load complete on a very unoptimized server).
865+
// Reasonable limit: one SReplacementTextures shouldn't be applied to hundreds of different TXDs.
866+
constexpr std::size_t kMaxTxdEntriesPerReplacement = 64;
867+
if (pReplacementTextures && pReplacementTextures->perTxdList.size() >= kMaxTxdEntriesPerReplacement)
868+
{
869+
// Defensive check for pInfo validity before accessing its members
870+
if (!pInfo)
871+
{
872+
UnregisterModelUsage(*pReplacementTextures, usModelId);
873+
return false;
874+
}
875+
876+
// Check if reusing existing TXD (allowed) or creating new entry (blocked at limit)
877+
if (auto existingIt = pReplacementTextures->perTxdIndexLookup.find(pInfo->usTxdId); existingIt == pReplacementTextures->perTxdIndexLookup.end())
878+
{
879+
// Would create new entry but already at limit - reject to prevent memory exhaustion
880+
UnregisterModelUsage(*pReplacementTextures, usModelId);
881+
return false;
882+
}
883+
}
884+
796885
bool isNewTxdEntry = false;
797886
SReplacementTextures::SPerTxd* pPerTxdInfo = RegisterTxdUsage(*pReplacementTextures, pInfo->usTxdId, isNewTxdEntry);
798887
dassert(pPerTxdInfo); // RegisterTxdUsage always returns non-null
@@ -889,7 +978,7 @@ bool CRenderWareSA::ModelInfoTXDAddTextures(SReplacementTextures* pReplacementTe
889978
const std::string_view replacementName = GetTextureNameView(pNewTexture);
890979

891980
RwTexture* pExistingTexture = LookupOriginalTexture(*pInfo, replacementName);
892-
if (!pExistingTexture && bHasValidTxd && !replacementName.empty())
981+
if (!pExistingTexture && bHasValidTxd && !std::empty(replacementName))
893982
{
894983
// Create null-terminated name for safe call
895984
TextureIndex::Name safeName;
@@ -935,7 +1024,7 @@ bool CRenderWareSA::ModelInfoTXDAddTextures(SReplacementTextures* pReplacementTe
9351024
pInfo->usedByReplacements.push_back(pReplacementTextures);
9361025

9371026
ValidateUsageTracking(*pReplacementTextures);
938-
return !pPerTxdInfo->usingTextures.empty();
1027+
return !std::empty(pPerTxdInfo->usingTextures);
9391028
}
9401029

9411030
////////////////////////////////////////////////////////////////
@@ -947,7 +1036,11 @@ bool CRenderWareSA::ModelInfoTXDAddTextures(SReplacementTextures* pReplacementTe
9471036
////////////////////////////////////////////////////////////////
9481037
void CRenderWareSA::ModelInfoTXDRemoveTextures(SReplacementTextures* pReplacementTextures)
9491038
{
950-
std::unique_lock<std::recursive_mutex> guard(ms_ModelTexturesInfoMutex);
1039+
// Validate input pointer first
1040+
if (!pReplacementTextures)
1041+
return;
1042+
1043+
std::scoped_lock guard(ms_ModelTexturesInfoMutex);
9511044
// For each using txd
9521045
for (std::size_t i = 0; i < pReplacementTextures->perTxdList.size(); ++i)
9531046
{
@@ -1022,21 +1115,19 @@ void CRenderWareSA::ModelInfoTXDRemoveTextures(SReplacementTextures* pReplacemen
10221115
{
10231116
ListRemove(pInfo->usedByReplacements, pReplacementTextures);
10241117

1025-
if (pInfo->usedByReplacements.empty())
1118+
if (std::empty(pInfo->usedByReplacements))
10261119
{
10271120
CTxdStore_RemoveRef(pInfo->usTxdId);
10281121
ms_ModelTexturesInfoMap.erase(usTxdId);
10291122
}
10301123
}
10311124
}
10321125

1033-
// Destroy replacement textures
1034-
for (std::size_t i = 0; i < pReplacementTextures->textures.size(); ++i)
1035-
{
1036-
RwTexture* pTexture = pReplacementTextures->textures[i];
1037-
DestroyTexture(pTexture);
1038-
}
1039-
pReplacementTextures->textures.clear();
1126+
// Destroy replacement textures - ownedTextures vector handles cleanup automatically via unique_ptr
1127+
// Textures that were added to TXDs (txd != nullptr) won't be destroyed by unique_ptr's deleter
1128+
// because they're managed by the TXD. Orphaned textures (txd == nullptr) are destroyed safely.
1129+
pReplacementTextures->ownedTextures.clear(); // Automatic cleanup via unique_ptr destructors
1130+
pReplacementTextures->textures.clear(); // Clear raw pointers
10401131

10411132
pReplacementTextures->perTxdList.clear();
10421133
pReplacementTextures->usedInTxdIds.clear();
@@ -1058,11 +1149,11 @@ void CRenderWareSA::ModelInfoTXDRemoveTextures(SReplacementTextures* pReplacemen
10581149
/////////////////////////////////////////////////////////////
10591150
void CRenderWareSA::ModelInfoTXDCleanupOrphanedEntries()
10601151
{
1061-
std::unique_lock lock(ms_ModelTexturesInfoMutex);
1152+
std::scoped_lock lock(ms_ModelTexturesInfoMutex);
10621153

10631154
for (auto it = ms_ModelTexturesInfoMap.begin(); it != ms_ModelTexturesInfoMap.end();)
10641155
{
1065-
if (it->second.usedByReplacements.empty())
1156+
if (std::empty(it->second.usedByReplacements))
10661157
{
10671158
CTxdStore_RemoveRef(it->first);
10681159
it = ms_ModelTexturesInfoMap.erase(it);

Client/game_sa/CRenderWareSA.cpp

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -887,6 +887,9 @@ void CRenderWareSA::GetTxdTextures(std::vector<RwTexture*>& outTextureList, usho
887887
if (!pTXD)
888888
return;
889889

890+
if (!SharedUtil::IsReadablePointer(pTXD, sizeof(*pTXD)))
891+
return;
892+
890893
GetTxdTextures(outTextureList, pTXD);
891894
}
892895

@@ -902,6 +905,32 @@ void CRenderWareSA::GetTxdTextures(std::vector<RwTexture*>& outTextureList, RwTe
902905
if (!pTXD)
903906
return;
904907

908+
// Validate TXD structure is readable (includes textures member)
909+
if (!SharedUtil::IsReadablePointer(pTXD, sizeof(*pTXD)))
910+
return;
911+
912+
// Validate the linked list structure to prevent crash at 0x007F374A
913+
// The crash occurs when next pointer is invalid and gets deref'd during iteration
914+
RwListEntry* firstNode = pTXD->textures.root.next;
915+
if (!SharedUtil::IsReadablePointer(firstNode, sizeof(RwListEntry)))
916+
return;
917+
918+
// Check for empty list (next points back to root - valid case)
919+
if (firstNode == &pTXD->textures.root)
920+
return; // Empty TXD is valid
921+
922+
// Validate the first texture node structure
923+
// The texture pointer is at (node - offsetof(RwTexture, TXDList))
924+
// which is (node - 8) since TXDList is at offset 8 in RwTexture
925+
RwTexture* firstTexture = (RwTexture*)((char*)firstNode - 8);
926+
if (!SharedUtil::IsReadablePointer(firstTexture, sizeof(RwTexture)))
927+
return;
928+
929+
// Validate that first node's next pointer is also readable
930+
// This catches most corruption cases where the list is broken
931+
if (!SharedUtil::IsReadablePointer(firstNode->next, sizeof(RwListEntry)))
932+
return;
933+
905934
constexpr std::size_t kMaxReasonableTextures = 8192;
906935
if (outTextureList.size() >= kMaxReasonableTextures)
907936
{
@@ -923,14 +952,22 @@ void CRenderWareSA::GetTxdTextures(std::vector<RwTexture*>& outTextureList, RwTe
923952
////////////////////////////////////////////////////////////////
924953
bool CRenderWareSA::StaticGetTextureCB(RwTexture* texture, std::vector<RwTexture*>* pTextureList)
925954
{
955+
// Fast null check before any heavy validation
926956
if (!texture || !pTextureList)
927957
return false;
928958

929-
// Sanity check: prevent excessive allocations from corrupted/malicious TXDs
959+
// Prevent excessive allocations from corrupted TXDs
930960
constexpr std::size_t kMaxReasonableTextures = 8192;
931961
if (pTextureList->size() >= kMaxReasonableTextures)
932962
return false; // Stop enumeration
933963

964+
// Note: We don't validate readability here for performance reasons.
965+
// The upfront validation in GetTxdTextures catches the first two nodes,
966+
// and the SA function crashes before calling this callback if the
967+
// linked list is corrupted mid-iteration. If we reach here with a bad
968+
// pointer, we'll crash in push_back, but that's acceptable vs the cost
969+
// of VirtualQuery on every texture in every TXD.
970+
934971
pTextureList->push_back(texture);
935972
return true;
936973
}

0 commit comments

Comments
 (0)