Skip to content

Commit f60d9c5

Browse files
committed
Fix race conditions in Notification by passing change and message as parameters instead of using shared state
1 parent ae92e7d commit f60d9c5

1 file changed

Lines changed: 25 additions & 9 deletions

File tree

src/Configuration/Notification.cs

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -147,18 +147,27 @@ internal void QueueRequest(TriggerType trigger, ChangeInfo change)
147147
/// <param name="change">
148148
/// Information about the file change to use for this notification. If null, uses the stored Change property.
149149
/// </param>
150+
/// <param name="message">
151+
/// Optional message to send. If null, uses the accumulated _message buffer.
152+
/// </param>
150153
/// <exception cref="InvalidOperationException">
151154
/// Thrown when the URL is null or empty.
152155
/// </exception>
153156
/// <exception cref="UriFormatException">
154157
/// Thrown when the URL is not in a valid format.
155158
/// </exception>
156-
internal async Task<Response?> SendAsync(ChangeInfo? change = null)
159+
internal async Task<Response?> SendAsync(ChangeInfo? change = null, string? message = null)
157160
{
158-
// If there isn't a message to be sent, then just return
159-
if (_message == null || _message.Length <= 0)
161+
// Determine which message to use
162+
string? messageToSend = message;
163+
if (string.IsNullOrEmpty(messageToSend))
160164
{
161-
return null;
165+
// Fall back to the accumulated _message buffer (for queued notifications)
166+
if (_message == null || _message.Length <= 0)
167+
{
168+
return null;
169+
}
170+
messageToSend = _message.ToString();
162171
}
163172

164173
// Use provided change or fall back to stored Change property
@@ -181,7 +190,7 @@ internal void QueueRequest(TriggerType trigger, ChangeInfo change)
181190
string? content = string.Empty;
182191
if (Data.Body != null)
183192
{
184-
content = Data.Body.Replace("[message]", _message.ToString(), StringComparison.OrdinalIgnoreCase);
193+
content = Data.Body.Replace("[message]", messageToSend, StringComparison.OrdinalIgnoreCase);
185194

186195
if (changeToUse != null)
187196
{
@@ -204,7 +213,12 @@ await Request.SendAsync(
204213
content,
205214
Data.MimeType).ConfigureAwait(false);
206215

207-
_message.Clear();
216+
// Only clear the shared _message buffer if we used it (queued notification path)
217+
if (string.IsNullOrEmpty(message))
218+
{
219+
_message.Clear();
220+
}
221+
208222
return response;
209223
}
210224

@@ -351,14 +365,16 @@ public override void Run(ChangeInfo change, TriggerType trigger)
351365

352366
// Store change in the shared property for backwards compatibility with QueueRequest
353367
Change = change;
354-
_message.Append(GetMessageString(change));
368+
369+
// Build message locally to avoid race conditions with concurrent executions
370+
string message = GetMessageString(change);
355371

356372
try
357373
{
358-
// Pass change as parameter to avoid race conditions with concurrent executions
374+
// Pass both change and message as parameters to avoid race conditions
359375
// Use GetAwaiter().GetResult() instead of .Result to avoid
360376
// AggregateException wrapping and potential deadlocks
361-
Response? response = SendAsync(change).GetAwaiter().GetResult();
377+
Response? response = SendAsync(change, message).GetAwaiter().GetResult();
362378
if (response != null)
363379
{
364380
Logger.WriteLine($"Response: {response.StatusCode}. URL: {response.Url}. Content: {response.Content}");

0 commit comments

Comments
 (0)