Skip to content

Commit 40a94aa

Browse files
b3ncr0wb3ncr0w
committed
Add deadlock protection using TryEnter
1 parent 1d44416 commit 40a94aa

File tree

2 files changed

+185
-49
lines changed

2 files changed

+185
-49
lines changed

‎Runtime/Scripts/Issue.cs‎

Lines changed: 57 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -215,19 +215,51 @@ public IEnumerator Publish(bool emailMyReport)
215215
// Helper method called after media upload completes
216216
private IEnumerator MarkMediaCompleteAndTryPublish()
217217
{
218-
lock (this)
218+
bool lockTaken = false;
219+
try
219220
{
220-
_mediaUploadComplete = true;
221+
System.Threading.Monitor.TryEnter(this, 1000, ref lockTaken); // 1 second timeout
222+
if (lockTaken)
223+
{
224+
_mediaUploadComplete = true;
225+
}
226+
else
227+
{
228+
Debug.LogWarning("Failed to acquire lock for MarkMediaCompleteAndTryPublish within timeout");
229+
}
230+
}
231+
finally
232+
{
233+
if (lockTaken)
234+
{
235+
System.Threading.Monitor.Exit(this);
236+
}
221237
}
222238
yield return CheckAndPublishIfReady();
223239
}
224240

225241
// Helper method called by the public Publish() method
226242
private IEnumerator RequestPublishAndTryPublish()
227243
{
228-
lock (this)
244+
bool lockTaken = false;
245+
try
229246
{
230-
_publishRequested = true;
247+
System.Threading.Monitor.TryEnter(this, 1000, ref lockTaken); // 1 second timeout
248+
if (lockTaken)
249+
{
250+
_publishRequested = true;
251+
}
252+
else
253+
{
254+
Debug.LogWarning("Failed to acquire lock for RequestPublishAndTryPublish within timeout");
255+
}
256+
}
257+
finally
258+
{
259+
if (lockTaken)
260+
{
261+
System.Threading.Monitor.Exit(this);
262+
}
231263
}
232264
yield return CheckAndPublishIfReady();
233265
}
@@ -236,13 +268,29 @@ private IEnumerator RequestPublishAndTryPublish()
236268
private IEnumerator CheckAndPublishIfReady()
237269
{
238270
bool shouldPublish = false;
239-
lock (this)
271+
bool lockTaken = false;
272+
try
273+
{
274+
System.Threading.Monitor.TryEnter(this, 1000, ref lockTaken); // 1 second timeout
275+
if (lockTaken)
276+
{
277+
// Check if publish is requested, media is done, not already published, and we have the necessary ID/token
278+
if (_publishRequested && _mediaUploadComplete && !_isPublished && !string.IsNullOrEmpty(Id) && !string.IsNullOrEmpty(_updateIssueAuthToken))
279+
{
280+
shouldPublish = true;
281+
_isPublished = true; // Prevent duplicate publish attempts
282+
}
283+
}
284+
else
285+
{
286+
Debug.LogWarning("Failed to acquire lock for CheckAndPublishIfReady within timeout");
287+
}
288+
}
289+
finally
240290
{
241-
// Check if publish is requested, media is done, not already published, and we have the necessary ID/token
242-
if (_publishRequested && _mediaUploadComplete && !_isPublished && !string.IsNullOrEmpty(Id) && !string.IsNullOrEmpty(_updateIssueAuthToken))
291+
if (lockTaken)
243292
{
244-
shouldPublish = true;
245-
_isPublished = true; // Prevent duplicate publish attempts
293+
System.Threading.Monitor.Exit(this);
246294
}
247295
}
248296

‎Runtime/Scripts/Logger.cs‎

Lines changed: 128 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -68,23 +68,40 @@ private void WriteToLog(string log)
6868
{
6969
if (disposed || writer == null) return;
7070

71-
lock (lockObject)
71+
bool lockTaken = false;
72+
try
7273
{
73-
try
74+
System.Threading.Monitor.TryEnter(lockObject, 500, ref lockTaken); // 500ms timeout
75+
if (lockTaken)
7476
{
75-
logBuffer.Add(log);
76-
77-
bool shouldFlush = logBuffer.Count >= BUFFER_SIZE ||
78-
(DateTime.UtcNow - lastFlushTime) >= FLUSH_INTERVAL;
79-
80-
if (shouldFlush)
77+
try
78+
{
79+
logBuffer.Add(log);
80+
81+
bool shouldFlush = logBuffer.Count >= BUFFER_SIZE ||
82+
(DateTime.UtcNow - lastFlushTime) >= FLUSH_INTERVAL;
83+
84+
if (shouldFlush)
85+
{
86+
FlushBuffer();
87+
}
88+
}
89+
catch (Exception e)
8190
{
82-
FlushBuffer();
91+
Debug.LogError("Error buffering log: " + e.Message);
8392
}
8493
}
85-
catch (Exception e)
94+
else
8695
{
87-
Debug.LogError("Error buffering log: " + e.Message);
96+
// If we can't acquire the lock, skip this log entry to prevent deadlock
97+
Debug.LogWarning("Failed to acquire lock for WriteToLog within timeout, skipping log entry");
98+
}
99+
}
100+
finally
101+
{
102+
if (lockTaken)
103+
{
104+
System.Threading.Monitor.Exit(lockObject);
88105
}
89106
}
90107
}
@@ -118,41 +135,89 @@ private void FlushBuffer()
118135

119136
public void ForceFlush()
120137
{
121-
lock (lockObject)
138+
bool lockTaken = false;
139+
try
140+
{
141+
System.Threading.Monitor.TryEnter(lockObject, 1000, ref lockTaken); // 1 second timeout
142+
if (lockTaken)
143+
{
144+
FlushBuffer();
145+
}
146+
else
147+
{
148+
Debug.LogWarning("Failed to acquire lock for ForceFlush within timeout");
149+
}
150+
}
151+
finally
122152
{
123-
FlushBuffer();
153+
if (lockTaken)
154+
{
155+
System.Threading.Monitor.Exit(lockObject);
156+
}
124157
}
125158
}
126159

127160
public void PauseLogging()
128161
{
129-
lock (lockObject)
162+
bool lockTaken = false;
163+
try
164+
{
165+
System.Threading.Monitor.TryEnter(lockObject, 1000, ref lockTaken); // 1 second timeout
166+
if (lockTaken)
167+
{
168+
FlushBuffer();
169+
writer?.Dispose();
170+
fileStream?.Dispose();
171+
writer = null;
172+
fileStream = null;
173+
}
174+
else
175+
{
176+
Debug.LogWarning("Failed to acquire lock for PauseLogging within timeout");
177+
}
178+
}
179+
finally
130180
{
131-
FlushBuffer();
132-
writer?.Dispose();
133-
fileStream?.Dispose();
134-
writer = null;
135-
fileStream = null;
181+
if (lockTaken)
182+
{
183+
System.Threading.Monitor.Exit(lockObject);
184+
}
136185
}
137186
}
138187

139188
public void ResumeLogging()
140189
{
141-
lock (lockObject)
190+
bool lockTaken = false;
191+
try
142192
{
143-
if (writer == null && fileStream == null)
193+
System.Threading.Monitor.TryEnter(lockObject, 1000, ref lockTaken); // 1 second timeout
194+
if (lockTaken)
144195
{
145-
try
146-
{
147-
// Append to existing file instead of recreating it
148-
fileStream = new FileStream(_logPath, FileMode.Append, FileAccess.Write, FileShare.ReadWrite);
149-
writer = new StreamWriter(fileStream) { AutoFlush = false };
150-
}
151-
catch (Exception e)
196+
if (writer == null && fileStream == null)
152197
{
153-
Debug.LogError("Error resuming log file stream: " + e.Message);
198+
try
199+
{
200+
// Append to existing file instead of recreating it
201+
fileStream = new FileStream(_logPath, FileMode.Append, FileAccess.Write, FileShare.ReadWrite);
202+
writer = new StreamWriter(fileStream) { AutoFlush = false };
203+
}
204+
catch (Exception e)
205+
{
206+
Debug.LogError("Error resuming log file stream: " + e.Message);
207+
}
154208
}
155209
}
210+
else
211+
{
212+
Debug.LogWarning("Failed to acquire lock for ResumeLogging within timeout");
213+
}
214+
}
215+
finally
216+
{
217+
if (lockTaken)
218+
{
219+
System.Threading.Monitor.Exit(lockObject);
220+
}
156221
}
157222
}
158223

@@ -206,18 +271,41 @@ public void Dispose()
206271

207272
Application.logMessageReceivedThreaded -= UnityLogHandler;
208273

209-
lock (lockObject)
274+
bool lockTaken = false;
275+
try
210276
{
211-
FlushBuffer();
212-
213-
writer?.Dispose();
214-
fileStream?.Dispose();
215-
216-
writer = null;
217-
fileStream = null;
218-
logBuffer?.Clear();
219-
220-
disposed = true;
277+
System.Threading.Monitor.TryEnter(lockObject, 2000, ref lockTaken); // 2 second timeout for disposal
278+
if (lockTaken)
279+
{
280+
FlushBuffer();
281+
282+
writer?.Dispose();
283+
fileStream?.Dispose();
284+
285+
writer = null;
286+
fileStream = null;
287+
logBuffer?.Clear();
288+
289+
disposed = true;
290+
}
291+
else
292+
{
293+
Debug.LogWarning("Failed to acquire lock for Dispose within timeout, forcing disposal");
294+
// Force disposal even without lock to prevent resource leaks
295+
writer?.Dispose();
296+
fileStream?.Dispose();
297+
writer = null;
298+
fileStream = null;
299+
logBuffer?.Clear();
300+
disposed = true;
301+
}
302+
}
303+
finally
304+
{
305+
if (lockTaken)
306+
{
307+
System.Threading.Monitor.Exit(lockObject);
308+
}
221309
}
222310
}
223311

0 commit comments

Comments
 (0)