Skip to content

Commit 8e3ceb9

Browse files
AnakinRaWAnakinRaW
committed
fix potential memory leaks because of undisposed VSB
1 parent b873c45 commit 8e3ceb9

4 files changed

Lines changed: 117 additions & 65 deletions

File tree

‎src/PetroglyphTools/PG.StarWarsGame.Engine/IO/MultiPassRepository.cs‎

Lines changed: 37 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -32,16 +32,22 @@ public bool FileExists(string filePath, bool megFileOnly, out bool inMeg, [NotNu
3232
{
3333
var multiPassSb = new ValueStringBuilder(stackalloc char[PGConstants.MaxMegEntryPathLength]);
3434
var destinationSb = new ValueStringBuilder(stackalloc char[PGConstants.MaxMegEntryPathLength]);
35-
var result = MultiPassAction(filePath, ref multiPassSb, ref destinationSb, megFileOnly);
36-
var fileFound = result.FileFound;
37-
inMeg = result.InMeg;
38-
if (!fileFound)
39-
actualFilePath = null;
40-
else
41-
actualFilePath = result.InMeg ? result.MegDataEntryReference.Path : result.FilePath.ToString();
42-
multiPassSb.Dispose();
43-
destinationSb.Dispose();
44-
return fileFound;
35+
try
36+
{
37+
var result = MultiPassAction(filePath, ref multiPassSb, ref destinationSb, megFileOnly);
38+
var fileFound = result.FileFound;
39+
inMeg = result.InMeg;
40+
if (!fileFound)
41+
actualFilePath = null;
42+
else
43+
actualFilePath = result.InMeg ? result.MegDataEntryReference.Path : result.FilePath.ToString();
44+
return fileFound;
45+
}
46+
finally
47+
{
48+
multiPassSb.Dispose();
49+
destinationSb.Dispose();
50+
}
4551
}
4652

4753
public bool FileExists(ReadOnlySpan<char> filePath, bool megFileOnly = false)
@@ -53,12 +59,17 @@ public bool FileExists(ReadOnlySpan<char> filePath, bool megFileOnly, out bool p
5359
{
5460
var multiPassSb = new ValueStringBuilder(stackalloc char[PGConstants.MaxMegEntryPathLength]);
5561
var destinationSb = new ValueStringBuilder(stackalloc char[PGConstants.MaxMegEntryPathLength]);
56-
var result = MultiPassAction(filePath, ref multiPassSb, ref destinationSb, megFileOnly);
57-
var fileFound = result.FileFound;
58-
pathTooLong = result.PathTooLong;
59-
multiPassSb.Dispose();
60-
destinationSb.Dispose();
61-
return fileFound;
62+
try
63+
{
64+
var result = MultiPassAction(filePath, ref multiPassSb, ref destinationSb, megFileOnly);
65+
pathTooLong = result.PathTooLong;
66+
return result.FileFound;
67+
}
68+
finally
69+
{
70+
multiPassSb.Dispose();
71+
destinationSb.Dispose();
72+
}
6273
}
6374

6475
private protected abstract FileFoundInfo MultiPassAction(
@@ -76,10 +87,15 @@ private protected abstract FileFoundInfo MultiPassAction(
7687
{
7788
var multiPassSb = new ValueStringBuilder(stackalloc char[PGConstants.MaxMegEntryPathLength]);
7889
var destinationSb = new ValueStringBuilder(stackalloc char[PGConstants.MaxMegEntryPathLength]);
79-
var fileFound = MultiPassAction(filePath, ref multiPassSb, ref destinationSb, megFileOnly);
80-
var result = BaseRepository.OpenFileCore(fileFound);
81-
multiPassSb.Dispose();
82-
destinationSb.Dispose();
83-
return result;
90+
try
91+
{
92+
var fileFound = MultiPassAction(filePath, ref multiPassSb, ref destinationSb, megFileOnly);
93+
return BaseRepository.OpenFileCore(fileFound);
94+
}
95+
finally
96+
{
97+
multiPassSb.Dispose();
98+
destinationSb.Dispose();
99+
}
84100
}
85101
}

‎src/PetroglyphTools/PG.StarWarsGame.Engine/IO/Repositories/GameRepository.Files.cs‎

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ public bool FileExists(string filePath, bool megFileOnly = false)
3232
public bool FileExists(string filePath, bool megFileOnly, out bool inMeg, [NotNullWhen(true)] out string? actualFilePath)
3333
{
3434
var sb = new ValueStringBuilder(stackalloc char[PGConstants.MaxMegEntryPathLength]);
35+
try
36+
{
3537
var fileFound = FindFile(filePath, ref sb, megFileOnly);
3638
var fileExists = fileFound.FileFound;
3739
inMeg = fileFound.InMeg;
@@ -42,6 +44,11 @@ public bool FileExists(string filePath, bool megFileOnly, out bool inMeg, [NotNu
4244
sb.Dispose();
4345
return fileExists;
4446
}
47+
finally
48+
{
49+
sb.Dispose();
50+
}
51+
}
4552

4653
public bool FileExists(ReadOnlySpan<char> filePath, bool megFileOnly = false)
4754
{
@@ -51,12 +58,19 @@ public bool FileExists(ReadOnlySpan<char> filePath, bool megFileOnly = false)
5158
public bool FileExists(ReadOnlySpan<char> filePath, bool megFileOnly, out bool pathTooLong)
5259
{
5360
var sb = new ValueStringBuilder(stackalloc char[PGConstants.MaxMegEntryPathLength]);
61+
try
62+
{
5463
var fileFound = FindFile(filePath, ref sb, megFileOnly);
5564
var fileExists = fileFound.FileFound;
5665
pathTooLong = fileFound.PathTooLong;
5766
sb.Dispose();
5867
return fileExists;
5968
}
69+
finally
70+
{
71+
sb.Dispose();
72+
}
73+
}
6074

6175
public Stream OpenFile(string filePath, bool megFileOnly = false)
6276
{
@@ -80,10 +94,15 @@ public Stream OpenFile(ReadOnlySpan<char> filePath, bool megFileOnly = false)
8094
public Stream? TryOpenFile(ReadOnlySpan<char> filePath, bool megFileOnly)
8195
{
8296
var sb = new ValueStringBuilder(stackalloc char[PGConstants.MaxMegEntryPathLength]);
97+
try
98+
{
8399
var fileFoundInfo = FindFile(filePath, ref sb, megFileOnly);
84-
var fileStream = OpenFileCore(fileFoundInfo);
100+
return OpenFileCore(fileFoundInfo);
101+
}
102+
finally
103+
{
85104
sb.Dispose();
86-
return fileStream;
105+
}
87106
}
88107

89108
/// <summary>
@@ -107,13 +126,19 @@ protected FileFoundInfo GetFileInfoFromMasterMeg(ReadOnlySpan<char> filePath)
107126
}
108127

109128
var sb = new ValueStringBuilder(stackalloc char[PGConstants.MaxMegEntryPathLength]);
129+
Span<char> fileNameSpan = stackalloc char[PGConstants.MaxMegEntryPathLength];
130+
bool normalized;
131+
int length;
132+
try
133+
{
110134
sb.Append(filePath);
111135
PGFileSystem.NormalizePath(ref sb);
112-
113-
Span<char> fileNameSpan = stackalloc char[PGConstants.MaxMegEntryPathLength];
114-
115-
var normalized = _megPathNormalizer.TryNormalize(sb.AsSpan(), fileNameSpan, out var length);
136+
normalized = _megPathNormalizer.TryNormalize(sb.AsSpan(), fileNameSpan, out length);
137+
}
138+
finally
139+
{
116140
sb.Dispose();
141+
}
117142

118143
if (!normalized)
119144
return default;

‎src/PetroglyphTools/PG.StarWarsGame.Engine/Localization/GameLanguageManager.cs‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -123,24 +123,34 @@ public string LocalizeFileName(string fileName, LanguageType language, out bool
123123
}
124124

125125
var stringBuilder = new ValueStringBuilder(stackalloc char[PGConstants.MaxMegEntryPathLength]);
126-
LocalizeFileName(fileName.AsSpan(), language, ref stringBuilder, out localized);
127-
if (!localized)
126+
try
127+
{
128+
LocalizeFileName(fileName.AsSpan(), language, ref stringBuilder, out localized);
129+
if (!localized)
130+
return fileName;
131+
132+
Debug.Assert(stringBuilder.Length == fileName.Length);
133+
return stringBuilder.ToString();
134+
}
135+
finally
128136
{
129137
stringBuilder.Dispose();
130-
return fileName;
131138
}
132-
133-
Debug.Assert(stringBuilder.Length == fileName.Length);
134-
return stringBuilder.ToString();
135139
}
136140

137141
public int LocalizeFileName(ReadOnlySpan<char> fileName, LanguageType language, Span<char> destination, out bool localized)
138142
{
139143
var sb = new ValueStringBuilder(destination.Length);
140-
LocalizeFileName(fileName, language, ref sb, out localized);
141-
sb.TryCopyTo(destination, out var written);
142-
sb.Dispose();
143-
return written;
144+
try
145+
{
146+
LocalizeFileName(fileName, language, ref sb, out localized);
147+
sb.TryCopyTo(destination, out var written);
148+
return written;
149+
}
150+
finally
151+
{
152+
sb.Dispose();
153+
}
144154
}
145155

146156
public void LocalizeFileName(ReadOnlySpan<char> fileName, LanguageType language, ref ValueStringBuilder stringBuilder, out bool localized)

‎src/PetroglyphTools/PG.StarWarsGame.Engine/Rendering/PGRender.cs‎

Lines changed: 29 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -117,40 +117,41 @@ public AnimationCollection LoadAnimations(
117117
while (loadingNumberedAnimations)
118118
{
119119
var stringBuilder = new ValueStringBuilder(stringBuffer);
120-
121-
CreateAnimationFilePath(ref stringBuilder, modelFileName, animationData.Value, subIndex);
122-
var animationFilenameWithoutExtension =
123-
_pgFileSystem.GetFileNameWithoutExtension(stringBuilder.AsSpan());
124-
InsertPath(ref stringBuilder, directory);
125-
126-
if (stringBuilder.Length > PGConstants.MaxAnimationFileName)
127-
{
128-
var animFile = stringBuilder.AsSpan().ToString();
129-
errorReporter.Assert(
130-
EngineAssert.Create(EngineAssertKind.ValueOutOfRange, animFile, [],
131-
$"Cannot get animation file '{animFile}' , because animation file path is too long."));
132-
continue;
133-
}
134-
135120
try
136121
{
137-
var animationAsset = Load3DAsset(stringBuilder.AsSpan(), metadataOnly, throwsOnLoad);
138-
if (animationAsset is IAloAnimationFile animationFile)
122+
CreateAnimationFilePath(ref stringBuilder, modelFileName, animationData.Value, subIndex);
123+
var animationFilenameWithoutExtension =
124+
_pgFileSystem.GetFileNameWithoutExtension(stringBuilder.AsSpan());
125+
InsertPath(ref stringBuilder, directory);
126+
127+
if (stringBuilder.Length > PGConstants.MaxAnimationFileName)
139128
{
140-
loadingNumberedAnimations = true;
141-
var crc = _hashingService.GetCrc32(animationFilenameWithoutExtension,
142-
PGConstants.DefaultPGEncoding);
143-
animations.AddAnimation(animationData.Key, animationFile, crc);
129+
var animFile = stringBuilder.AsSpan().ToString();
130+
errorReporter.Assert(
131+
EngineAssert.Create(EngineAssertKind.ValueOutOfRange, animFile, [],
132+
$"Cannot get animation file '{animFile}' , because animation file path is too long."));
133+
continue;
144134
}
145-
else
135+
136+
try
146137
{
147-
loadingNumberedAnimations = false;
138+
var animationAsset = Load3DAsset(stringBuilder.AsSpan(), metadataOnly, throwsOnLoad);
139+
if (animationAsset is IAloAnimationFile animationFile)
140+
{
141+
loadingNumberedAnimations = true;
142+
var crc = _hashingService.GetCrc32(animationFilenameWithoutExtension,
143+
PGConstants.DefaultPGEncoding);
144+
animations.AddAnimation(animationData.Key, animationFile, crc);
145+
}
146+
else
147+
{
148+
loadingNumberedAnimations = false;
149+
}
150+
}
151+
catch (BinaryCorruptedException e)
152+
{
153+
corruptedAnimationHandler?.Invoke(e, animationData.Key, stringBuilder.AsSpan().ToString());
148154
}
149-
}
150-
catch (BinaryCorruptedException e)
151-
{
152-
// NB: Loading a corrupted animation does not break the loading of other numbered animations
153-
corruptedAnimationHandler?.Invoke(e, animationData.Key, stringBuilder.AsSpan().ToString());
154155
}
155156
finally
156157
{

0 commit comments

Comments
 (0)