src/NLog/Config/LoggingConfiguration.cs (1)

979-995: The async completion pattern correctly avoids potential deadlocks.

The StartAsyncTask invocation ensures the flush completion callback executes outside the target lock context, which is the core fix for the deadlock scenario.

Similar to the suggestion for AsyncHelpers.cs, the two branches could be unified since invoking with lastException works for both null and non-null cases.

,

♻️ Optional simplification

                 if (completed)
                 {
                     // The flush callback happens while holding target lock, so execute the flush completion asynchronously to avoid potential deadlocks.
+                    var flushException = lastException;
                     if (lastException != null)
-                    {
                         InternalLogger.Warn("Flush completed with errors");
-                        AsyncHelpers.StartAsyncTask(s =>
-                        {
-                            ((AsyncContinuation)s).Invoke(lastException);
-                        }, flushCompletion);
-                    }
                     else
-                    {
                         InternalLogger.Debug("Flush completed");
-                        AsyncHelpers.StartAsyncTask(s =>
-                        {
-                            ((AsyncContinuation)s).Invoke(null);
-                        }, flushCompletion);
-                    }
+                    AsyncHelpers.StartAsyncTask(s =>
+                    {
+                        ((AsyncContinuation)s).Invoke(flushException);
+                    }, flushCompletion);
                 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/NLog/Config/LoggingConfiguration.cs` around lines 979 - 995, The two
conditional branches that call AsyncHelpers.StartAsyncTask can be simplified:
remove the duplicate branches and call AsyncHelpers.StartAsyncTask once,
invoking ((AsyncContinuation)s).Invoke(lastException) so it works for both error
and success (lastException may be null); keep the existing InternalLogger
message (choose Warn if lastException != null else Debug) or optionally set the
log before the single StartAsyncTask call, and reuse the same
AsyncContinuation/flushCompletion invocation to avoid duplicated code (refer to
AsyncHelpers.StartAsyncTask, AsyncContinuation, lastException, and
flushCompletion in LoggingConfiguration.cs).

src/NLog/Common/AsyncHelpers.cs (1)

221-237: Consider simplifying the two branches into a single async call.

Both branches perform the same operation—invoking the continuation with combinedException. Since combinedException is already null in the first branch, you can unify them.

♻️ Proposed simplification

                     var combinedException = GetCombinedException(exceptions ?? ArrayHelper.Empty<Exception>());
 
                         // The flush callback happens while holding target lock, so execute the flush completion asynchronously to avoid potential deadlocks.
-                        if (combinedException is null)
-                        {
-                            AsyncHelpers.StartAsyncTask(s =>
-                            {
-                                ((AsyncContinuation)s).Invoke(null);
-                            }, asyncContinuation);
-                        }
-                        else
+                        var flushException = combinedException;
+                        AsyncHelpers.StartAsyncTask(s =>
                         {
-                            AsyncHelpers.StartAsyncTask(s =>
-                            {
-                                ((AsyncContinuation)s).Invoke(combinedException);
-                            }, asyncContinuation);
-                        }
+                            ((AsyncContinuation)s).Invoke(flushException);
+                        }, asyncContinuation);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/NLog/Common/AsyncHelpers.cs` around lines 221 - 237, The two branches
calling AsyncHelpers.StartAsyncTask can be merged: remove the if/else and call
AsyncHelpers.StartAsyncTask once, passing a lambda that casts s to
AsyncContinuation and invokes it with combinedException (which may be null) and
passing asyncContinuation as the state; update the block around
combinedException, AsyncHelpers.StartAsyncTask, and AsyncContinuation to use
this single unified call.

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro