Core/Vmaps: Fixed use after free and deadlocks when loading a model file fails

Port From (https://github.com/TrinityCore/TrinityCore/commit/d05dbaaecbd4bf7e1c0fd32d0fef34341b4067f6)
This commit is contained in:
Hondacrx
2024-08-31 18:44:32 -04:00
parent a9a28c889e
commit 8b991e1a06
3 changed files with 29 additions and 26 deletions
@@ -183,13 +183,15 @@ namespace Game.Collision
public WorldModel AcquireModelInstance(string filename) public WorldModel AcquireModelInstance(string filename)
{ {
ManagedModel worldmodel; // this is intentionally declared before lock so that it is destroyed after it to prevent deadlocks in releaseModelInstance
lock (LoadedModelFilesLock) lock (LoadedModelFilesLock)
{ {
filename = filename.TrimEnd('\0'); filename = filename.TrimEnd('\0');
if (iLoadedModelFiles.TryGetValue(filename, out ManagedModel worldmodel)) if (iLoadedModelFiles.TryGetValue(filename, out worldmodel))
return worldmodel.Model; return worldmodel.Model;
var model = new ManagedModel(); var model = new ManagedModel(filename);
if (!model.Model.ReadFile(VMapPath + filename)) if (!model.Model.ReadFile(VMapPath + filename))
{ {
Log.outError(LogFilter.Server, $"VMapManager: could not load '{filename}'"); Log.outError(LogFilter.Server, $"VMapManager: could not load '{filename}'");
@@ -198,8 +200,6 @@ namespace Game.Collision
Log.outDebug(LogFilter.Maps, $"VMapManager: loading file '{filename}'"); Log.outDebug(LogFilter.Maps, $"VMapManager: loading file '{filename}'");
worldmodel.Model.SetName(filename);
model = worldmodel; model = worldmodel;
iLoadedModelFiles.Add(filename, model); iLoadedModelFiles.Add(filename, model);
@@ -212,14 +212,15 @@ namespace Game.Collision
lock (LoadedModelFilesLock) lock (LoadedModelFilesLock)
{ {
filename = filename.TrimEnd('\0'); filename = filename.TrimEnd('\0');
Log.outDebug(LogFilter.Maps, $"VMapManager: unloading file '{filename}'");
var erased = iLoadedModelFiles.Remove(filename); var erased = iLoadedModelFiles.Remove(filename);
if (!erased) if (!erased)
{ {
Log.outError(LogFilter.Server, $"VMapManager: trying to unload non-loaded file '{filename}'"); Log.outError(LogFilter.Server, $"VMapManager: trying to unload non-loaded file '{filename}'");
return; return;
} }
Log.outDebug(LogFilter.Maps, $"VMapManager: unloading file '{filename}'");
} }
} }
@@ -271,10 +272,16 @@ namespace Game.Collision
public class ManagedModel public class ManagedModel
{ {
public WorldModel Model; public WorldModel Model;
string _name; // valid only while model is held in VMapManager2::iLoadedModelFiles
public ManagedModel(string name)
{
_name = name;
}
~ManagedModel() ~ManagedModel()
{ {
Global.VMapMgr.ReleaseModelInstance(Model.GetName()); Global.VMapMgr.ReleaseModelInstance(_name);
} }
} }
+5 -5
View File
@@ -123,6 +123,7 @@ namespace Game.Collision
if (referencedVal >= iTreeValues.Length) if (referencedVal >= iTreeValues.Length)
{ {
Log.outError(LogFilter.Maps, $"StaticMapTree::LoadMapTile() : invalid tree element ({referencedVal}/{iTreeValues.Length}) referenced in tile {fileResult.Name}"); Log.outError(LogFilter.Maps, $"StaticMapTree::LoadMapTile() : invalid tree element ({referencedVal}/{iTreeValues.Length}) referenced in tile {fileResult.Name}");
result = LoadResult.ReadFromFileFailed;
continue; continue;
} }
@@ -175,15 +176,15 @@ namespace Game.Collision
for (uint i = 0; i < numSpawns && result; ++i) for (uint i = 0; i < numSpawns && result; ++i)
{ {
// read model spawns // read model spawns
ModelSpawn spawn; if (!ModelSpawn.ReadFromFile(reader, out ModelSpawn spawn))
result = ModelSpawn.ReadFromFile(reader, out spawn); break;
if (result)
{
// update tree // update tree
int referencedNode = spawnIndicesReader.ReadInt32(); int referencedNode = spawnIndicesReader.ReadInt32();
if (referencedNode >= iTreeValues.Length) if (referencedNode >= iTreeValues.Length)
{ {
Log.outError(LogFilter.Maps, $"StaticMapTree::LoadMapTile() : invalid tree element ({referencedNode}/{iTreeValues.Length}) referenced in tile {fileResult.Name}"); Log.outError(LogFilter.Maps, $"StaticMapTree::LoadMapTile() : invalid tree element ({referencedNode}/{iTreeValues.Length}) referenced in tile {fileResult.Name}");
result = false;
continue; continue;
} }
@@ -194,7 +195,6 @@ namespace Game.Collision
} }
} }
} }
}
iLoadedTiles.Remove(tileID); iLoadedTiles.Remove(tileID);
} }
@@ -331,7 +331,6 @@ namespace Game.Collision
uint RootWMOID; uint RootWMOID;
List<GroupModel> groupModels = new(); List<GroupModel> groupModels = new();
BIH groupTree = new(); BIH groupTree = new();
string name; // valid only while model is held in VMapManager2::iLoadedModelFiles
public override bool IntersectRay(Ray ray, ref float distance, bool stopAtFirstHit, ModelIgnoreFlags ignoreFlags) public override bool IntersectRay(Ray ray, ref float distance, bool stopAtFirstHit, ModelIgnoreFlags ignoreFlags)
{ {
@@ -423,9 +422,6 @@ namespace Game.Collision
return groupTree.ReadFromFile(reader); return groupTree.ReadFromFile(reader);
} }
public string GetName() { return name; }
public void SetName(string newName) { name = newName; }
public bool IsM2() { return Flags.HasFlag(ModelFlags.IsM2); } public bool IsM2() { return Flags.HasFlag(ModelFlags.IsM2); }
} }