Skip to content

Commit fe8de45

Browse files
committed
Trying to work around a reported race condition on opening DLL files for reading, probably due MM being instantiated twice or, perhaps, a file handler leak due forgetting to call Dispose when opening file streams?
In a way or another, refer to this post of mine for reference: https://forum.kerbalspaceprogram.com/index.php?/topic/50533-18x-112x-module-manager-422-june-18th-2022-the-heatwave-edition/page/302/#comment-4283448
1 parent 45ee59c commit fe8de45

File tree

1 file changed

+43
-5
lines changed

1 file changed

+43
-5
lines changed

Source/ModuleManager/Utils/FileUtils.cs

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,10 @@ You should have received a copy of the GNU General Public License 3.0
1717
using System;
1818
using System.IO;
1919
using System.Security.Cryptography;
20+
using System.Threading;
21+
2022
using ModuleManager.Extensions;
23+
using Log = ModuleManager.Logging.ModLogger;
2124

2225
namespace ModuleManager.Utils
2326
{
@@ -29,12 +32,47 @@ public static string FileSHA(string filename)
2932

3033
byte[] data = null;
3134

32-
using (SHA256 sha = SHA256.Create())
3335
{
34-
using (FileStream fs = File.Open(filename, FileMode.Open, FileAccess.Read))
35-
{
36-
data = sha.ComputeHash(fs);
37-
}
36+
/*
37+
* This one is hairy. Absolutely hairy.
38+
*
39+
* By some reason, pretty powerful rigs are getting screwed by what I think is a race condition (perhaps induced by MM being
40+
* instantiated twice somehow), with the file failing being opened due it being already opened by someone else.
41+
*
42+
* It was postulated that KSP would be the one keeping these files opened. I think it's not because by the time
43+
* Module Manager is started up, KSP had already loaded and resolved every single DLL (and its dependencies) and, so,
44+
* there will be just not a single logic reason to keep them opened in memory - unless KSP is not calling the Dispose
45+
* or not using the `using` construction and, so, we have a file handlers leaking on this damned thing.
46+
*
47+
* In a way or another, the proposed solution on pull/request 180 to the upstream is, IMHO, completely out of the line.
48+
* **WE JUST DON'T** open executable files with Writing privileges, **POINT**. At very least, this will prevent anti-virus
49+
* software from being triggered on us, avoiding slowing down KSP's file accesses.
50+
*
51+
* So I will not use `FileShare.ReadWrite` no matter what. I terminantly refuse to do so.
52+
*
53+
* See:
54+
* + https://github.com/sarbian/ModuleManager/pull/180
55+
* + https://forum.kerbalspaceprogram.com/index.php?/topic/50533-18x-112x-module-manager-422-june-18th-2022-the-heatwave-edition/page/302/#comment-4283448
56+
*/
57+
Exception ex = null;
58+
int i = 8; // Max wait: 1 second
59+
while (i-- > 0) try
60+
{
61+
using (SHA256 sha = SHA256.Create())
62+
using (FileStream fs = File.Open(filename, FileMode.Open, FileAccess.Read, FileShare.Read))
63+
data = sha.ComputeHash(fs);
64+
break;
65+
}
66+
catch (Exception e)
67+
{
68+
ex = e;
69+
Log.LOG.detail("Error {0} on reading {1} on regressive count {2}!", e.Message, filename, i);
70+
GC.Collect(); // For the hypothesis KSP is leaking file handlers
71+
Thread.Sleep(125); // In milliseconds
72+
}
73+
if (0 == i && null != ex) throw ex;
74+
if (null != ex)
75+
Log.LOG.warn("File {0} suffered at least one Exception with message \"{1}\" while being processed. The problem was recovered, but this log was issued to mark the event.", filename, ex.Message);
3876
}
3977

4078
return data.ToHex();

0 commit comments

Comments
 (0)