Skip to content

Commit cc514ab

Browse files
SomficSomfic
committed
🔒️ improve file writing stability
semver: patch
1 parent 8a49f72 commit cc514ab

5 files changed

Lines changed: 266 additions & 21 deletions

File tree

‎EliteAPI.Tests/FileUtilsTests.cs‎

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
using System;
2+
using System.IO;
3+
using System.Threading;
4+
using System.Threading.Tasks;
5+
using EliteAPI.Utils;
6+
using FluentAssertions;
7+
8+
namespace EliteAPI.Tests;
9+
10+
public class FileUtilsTests
11+
{
12+
private string _testDir = null!;
13+
14+
[Before(Test)]
15+
public void Setup()
16+
{
17+
_testDir = Path.Combine(Path.GetTempPath(), $"EliteAPI.Tests.{Guid.NewGuid()}");
18+
Directory.CreateDirectory(_testDir);
19+
}
20+
21+
[After(Test)]
22+
public void Cleanup()
23+
{
24+
if (Directory.Exists(_testDir))
25+
Directory.Delete(_testDir, true);
26+
}
27+
28+
[Test]
29+
public void WriteWithRetry_WritesContentToFile()
30+
{
31+
var path = Path.Combine(_testDir, "test.txt");
32+
var content = "Hello, World!";
33+
34+
FileUtils.WriteWithRetry(path, content);
35+
36+
File.Exists(path).Should().BeTrue();
37+
File.ReadAllText(path).Should().Be(content);
38+
}
39+
40+
[Test]
41+
public void WriteWithRetry_OverwritesExistingFile()
42+
{
43+
var path = Path.Combine(_testDir, "test.txt");
44+
File.WriteAllText(path, "Old content");
45+
46+
FileUtils.WriteWithRetry(path, "New content");
47+
48+
File.ReadAllText(path).Should().Be("New content");
49+
}
50+
51+
[Test]
52+
public void WriteWithRetry_AllowsConcurrentReads()
53+
{
54+
var path = Path.Combine(_testDir, "test.txt");
55+
FileUtils.WriteWithRetry(path, "Initial content");
56+
57+
// Open file for reading with share
58+
using var readStream = new FileStream(path, FileMode.Open, FileAccess.Read, FileShare.ReadWrite);
59+
using var reader = new StreamReader(readStream);
60+
61+
// Should be able to write while file is open for reading
62+
var writeAction = () => FileUtils.WriteWithRetry(path, "Updated content");
63+
writeAction.Should().NotThrow();
64+
}
65+
66+
[Test]
67+
public async Task WriteWithRetry_RetriesOnTemporaryLock()
68+
{
69+
var path = Path.Combine(_testDir, "test.txt");
70+
FileUtils.WriteWithRetry(path, "Initial");
71+
72+
// Lock the file temporarily
73+
var lockReleased = false;
74+
var writeCompleted = false;
75+
76+
var lockTask = Task.Run(() =>
77+
{
78+
using var lockStream = new FileStream(path, FileMode.Open, FileAccess.ReadWrite, FileShare.None);
79+
Thread.Sleep(80); // Hold lock for 80ms (retry happens at 50ms intervals)
80+
lockReleased = true;
81+
});
82+
83+
// Start write after a small delay to ensure lock is acquired
84+
await Task.Delay(10);
85+
86+
var writeTask = Task.Run(() =>
87+
{
88+
FileUtils.WriteWithRetry(path, "After retry", maxRetries: 3, retryDelayMs: 50);
89+
writeCompleted = true;
90+
});
91+
92+
await Task.WhenAll(lockTask, writeTask);
93+
94+
lockReleased.Should().BeTrue();
95+
writeCompleted.Should().BeTrue();
96+
File.ReadAllText(path).Should().Be("After retry");
97+
}
98+
99+
[Test]
100+
public void WriteWithRetry_ThrowsAfterMaxRetries()
101+
{
102+
var path = Path.Combine(_testDir, "test.txt");
103+
FileUtils.WriteWithRetry(path, "Initial");
104+
105+
// Hold exclusive lock on the file
106+
using var lockStream = new FileStream(path, FileMode.Open, FileAccess.ReadWrite, FileShare.None);
107+
108+
// Should throw after retries are exhausted
109+
var writeAction = () => FileUtils.WriteWithRetry(path, "Should fail", maxRetries: 2, retryDelayMs: 10);
110+
writeAction.Should().Throw<IOException>();
111+
}
112+
113+
[Test]
114+
public async Task WriteWithRetry_HandlesMultipleConcurrentWriters()
115+
{
116+
var path = Path.Combine(_testDir, "test.txt");
117+
var writeCount = 10;
118+
var tasks = new Task[writeCount];
119+
120+
for (var i = 0; i < writeCount; i++)
121+
{
122+
var content = $"Content {i}";
123+
tasks[i] = Task.Run(() => FileUtils.WriteWithRetry(path, content));
124+
}
125+
126+
var allTasks = () => Task.WhenAll(tasks);
127+
await allTasks.Should().NotThrowAsync();
128+
129+
// File should exist and have content from one of the writes
130+
File.Exists(path).Should().BeTrue();
131+
File.ReadAllText(path).Should().StartWith("Content ");
132+
}
133+
134+
[Test]
135+
public void AppendWithRetry_AppendsContentToFile()
136+
{
137+
var path = Path.Combine(_testDir, "test.txt");
138+
File.WriteAllText(path, "Line 1\n");
139+
140+
FileUtils.AppendWithRetry(path, "Line 2\n");
141+
142+
File.ReadAllText(path).Should().Be("Line 1\nLine 2\n");
143+
}
144+
145+
[Test]
146+
public void AppendWithRetry_CreatesFileIfNotExists()
147+
{
148+
var path = Path.Combine(_testDir, "new.txt");
149+
150+
FileUtils.AppendWithRetry(path, "First line");
151+
152+
File.Exists(path).Should().BeTrue();
153+
File.ReadAllText(path).Should().Be("First line");
154+
}
155+
156+
[Test]
157+
public async Task AppendWithRetry_HandlesMultipleConcurrentAppends()
158+
{
159+
var path = Path.Combine(_testDir, "test.txt");
160+
File.WriteAllText(path, "");
161+
var appendCount = 10;
162+
var tasks = new Task[appendCount];
163+
164+
for (var i = 0; i < appendCount; i++)
165+
{
166+
var content = $"Line {i}\n";
167+
tasks[i] = Task.Run(() => FileUtils.AppendWithRetry(path, content));
168+
}
169+
170+
var allTasks = () => Task.WhenAll(tasks);
171+
await allTasks.Should().NotThrowAsync();
172+
173+
// File should have all lines (order may vary due to concurrency)
174+
var lines = File.ReadAllLines(path);
175+
lines.Should().HaveCount(appendCount);
176+
}
177+
}

‎EliteAPI/Utils/FileUtils.cs‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
using System;
2+
using System.IO;
3+
using System.Threading;
4+
5+
namespace EliteAPI.Utils;
6+
7+
public static class FileUtils
8+
{
9+
private static readonly object FileLock = new();
10+
11+
/// <summary>
12+
/// Writes content to a file with retry logic for handling temporary file locks.
13+
/// Uses a lock to prevent concurrent writes and FileShare.Read to allow readers.
14+
/// </summary>
15+
/// <param name="path">The file path to write to</param>
16+
/// <param name="content">The content to write</param>
17+
/// <param name="maxRetries">Maximum number of retry attempts (default: 3)</param>
18+
/// <param name="retryDelayMs">Delay between retries in milliseconds (default: 50)</param>
19+
/// <exception cref="IOException">Thrown if all retry attempts fail</exception>
20+
public static void WriteWithRetry(string path, string content, int maxRetries = 3, int retryDelayMs = 50)
21+
{
22+
lock (FileLock)
23+
{
24+
for (var i = 0; i < maxRetries; i++)
25+
{
26+
try
27+
{
28+
using var stream = new FileStream(path, FileMode.Create, FileAccess.Write, FileShare.Read);
29+
using var writer = new StreamWriter(stream);
30+
writer.Write(content);
31+
return;
32+
}
33+
catch (IOException) when (i < maxRetries - 1)
34+
{
35+
Thread.Sleep(retryDelayMs);
36+
}
37+
}
38+
}
39+
}
40+
41+
/// <summary>
42+
/// Appends content to a file with retry logic for handling temporary file locks.
43+
/// Uses a lock to prevent concurrent writes and FileShare.Read to allow readers.
44+
/// </summary>
45+
/// <param name="path">The file path to append to</param>
46+
/// <param name="content">The content to append</param>
47+
/// <param name="maxRetries">Maximum number of retry attempts (default: 3)</param>
48+
/// <param name="retryDelayMs">Delay between retries in milliseconds (default: 50)</param>
49+
/// <exception cref="IOException">Thrown if all retry attempts fail</exception>
50+
public static void AppendWithRetry(string path, string content, int maxRetries = 3, int retryDelayMs = 50)
51+
{
52+
lock (FileLock)
53+
{
54+
for (var i = 0; i < maxRetries; i++)
55+
{
56+
try
57+
{
58+
using var stream = new FileStream(path, FileMode.Append, FileAccess.Write, FileShare.Read);
59+
using var writer = new StreamWriter(stream);
60+
writer.Write(content);
61+
return;
62+
}
63+
catch (IOException) when (i < maxRetries - 1)
64+
{
65+
Thread.Sleep(retryDelayMs);
66+
}
67+
}
68+
}
69+
}
70+
}

‎EliteAPI/Utils/Logger.cs‎

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ namespace EliteAPI.Utils;
66

77
public static class Log
88
{
9-
private static readonly object _fileLock = new();
9+
private static readonly object _pathLock = new();
1010
private static readonly object _listenerLock = new();
1111
private static string? _logFilePath;
1212
private static readonly List<Action<LogMessage>> _listeners = [];
@@ -35,7 +35,7 @@ public static void SetLogFilePath(string path)
3535
if (!File.Exists(path))
3636
File.Create(path).Dispose();
3737

38-
lock (_fileLock)
38+
lock (_pathLock)
3939
{
4040
_logFilePath = path;
4141
}
@@ -122,25 +122,22 @@ public static void Write(LogLevel level, string message)
122122
var timestamp = DateTime.Now;
123123
var logEntry = $"[{timestamp:yyyy-MM-dd HH:mm:ss.fff}] [{level}] {message}";
124124

125-
// Write to file in a thread-safe manner
126-
lock (_fileLock)
125+
// Write to file with retry logic for handling temporary locks
126+
try
127127
{
128-
try
128+
var logPath = GetLogFilePath();
129+
var directory = Path.GetDirectoryName(logPath);
130+
if (!string.IsNullOrEmpty(directory) && !Directory.Exists(directory))
129131
{
130-
var logPath = GetLogFilePath();
131-
var directory = Path.GetDirectoryName(logPath);
132-
if (!string.IsNullOrEmpty(directory) && !Directory.Exists(directory))
133-
{
134-
Directory.CreateDirectory(directory);
135-
}
136-
137-
File.AppendAllText(logPath, logEntry + Environment.NewLine);
138-
}
139-
catch (Exception ex)
140-
{
141-
// If we can't write to the log file, at least write to console
142-
Console.WriteLine($"Failed to write to log file: {ex.Message}");
132+
Directory.CreateDirectory(directory);
143133
}
134+
135+
FileUtils.AppendWithRetry(logPath, logEntry + Environment.NewLine);
136+
}
137+
catch (Exception ex)
138+
{
139+
// If we can't write to the log file, at least write to console
140+
Console.WriteLine($"Failed to write to log file: {ex.Message}");
144141
}
145142

146143
// Notify listeners in a thread-safe manner

‎EliteVA/EliteVA.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public override async Task OnStart(IVoiceAttackProxy proxy)
5555
if (!Directory.Exists(Path.Combine(Dir, "Variables")))
5656
Directory.CreateDirectory(Path.Combine(Dir, "Variables"));
5757

58-
File.WriteAllText(Path.Combine(Dir, "Variables", "Keybindings.txt"), bindings.Select(b => $"{{TXT:EliteAPI.{b.Name}}}: {b.KeyCode}").Aggregate((a, b) => $"{a}\n{b}"));
58+
FileUtils.WriteWithRetry(Path.Combine(Dir, "Variables", "Keybindings.txt"), bindings.Select(b => $"{{TXT:EliteAPI.{b.Name}}}: {b.KeyCode}").Aggregate((a, b) => $"{a}\n{b}"));
5959

6060
proxy.Log.Write($"Applying {bindings.Count(b => !string.IsNullOrEmpty(b.KeyCode))} keybindings", VoiceAttackColor.Blue);
6161
});
@@ -91,7 +91,7 @@ public override async Task OnStart(IVoiceAttackProxy proxy)
9191
if (!Directory.Exists(Path.Combine(Dir, "Variables")))
9292
Directory.CreateDirectory(Path.Combine(Dir, "Variables"));
9393

94-
File.WriteAllText(Path.Combine(Dir, "Variables", $"{e.eventName}.txt"), paths.Select(p => $"{{{p.Type.ToDisplayType()}:{p.Path}}}: {p.Value}").Aggregate((a, b) => $"{a}\n{b}"));
94+
FileUtils.WriteWithRetry(Path.Combine(Dir, "Variables", $"{e.eventName}.txt"), paths.Select(p => $"{{{p.Type.ToDisplayType()}:{p.Path}}}: {p.Value}").Aggregate((a, b) => $"{a}\n{b}"));
9595
}
9696
});
9797

‎EliteVA/VoiceAttackPluginWrapper.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System;
22
using System.IO;
33
using System.Linq;
4+
using EliteAPI.Utils;
45
using EliteVA.Logging;
56

67
namespace EliteVA;
@@ -39,7 +40,7 @@ public static void VA_Init1(dynamic vaProxy)
3940
VoiceAttackPlugin.Instance.WriteToLog(VoiceAttackColor.Red, "Error during plugin initialisation. See logs for further information");
4041
var path = Path.Combine(VoiceAttackPlugin.Dir, "Logs", "STARTUP ERROR.log");
4142
Directory.CreateDirectory(Path.GetDirectoryName(path) ?? VoiceAttackPlugin.Dir);
42-
File.WriteAllText(path, e.ToString());
43+
FileUtils.WriteWithRetry(path, e.ToString());
4344
}
4445
}
4546

0 commit comments

Comments
 (0)