Skip to content
This repository was archived by the owner on Dec 13, 2018. It is now read-only.

Commit ab43352

Browse files
davidfowlBrennanConroy
authored andcommitted
Logging shouldn't throw after dispose
- Check complete adding before adding new messages to the list. - Added a test
1 parent 3004e1f commit ab43352

File tree

2 files changed

+36
-21
lines changed

2 files changed

+36
-21
lines changed

src/Microsoft.Extensions.Logging.Console/Internal/ConsoleLoggerProcessor.cs

+4-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,10 @@ public ConsoleLoggerProcessor()
2727

2828
public virtual void EnqueueMessage(LogMessageEntry message)
2929
{
30-
_messageQueue.Add(message);
30+
if (!_messageQueue.IsAddingCompleted)
31+
{
32+
_messageQueue.Add(message);
33+
}
3134
}
3235

3336
// for testing

test/Microsoft.Extensions.Logging.Test/ConsoleLoggerTest.cs

+32-20
Original file line numberDiff line numberDiff line change
@@ -769,12 +769,12 @@ public void WriteCore_NullMessageWithException()
769769
var expected = ex.ToString() + Environment.NewLine;
770770

771771
// Act
772-
logger.Log(LogLevel.Critical, 0, message, ex, (s,e) => s);
773-
logger.Log(LogLevel.Error, 0, message, ex, (s,e) => s);
774-
logger.Log(LogLevel.Warning, 0, message, ex, (s,e) => s);
775-
logger.Log(LogLevel.Information, 0, message, ex, (s,e) => s);
776-
logger.Log(LogLevel.Debug, 0, message, ex, (s,e) => s);
777-
logger.Log(LogLevel.Trace, 0, message, ex, (s,e) => s);
772+
logger.Log(LogLevel.Critical, 0, message, ex, (s, e) => s);
773+
logger.Log(LogLevel.Error, 0, message, ex, (s, e) => s);
774+
logger.Log(LogLevel.Warning, 0, message, ex, (s, e) => s);
775+
logger.Log(LogLevel.Information, 0, message, ex, (s, e) => s);
776+
logger.Log(LogLevel.Debug, 0, message, ex, (s, e) => s);
777+
logger.Log(LogLevel.Trace, 0, message, ex, (s, e) => s);
778778

779779
// Assert
780780
Assert.Equal(6, sink.Writes.Count);
@@ -796,12 +796,12 @@ public void WriteCore_MessageWithNullException()
796796
Exception ex = null;
797797

798798
// Act
799-
logger.Log(LogLevel.Critical, 0, _state, ex, (s,e) => s);
800-
logger.Log(LogLevel.Error, 0, _state, ex, (s,e) => s);
801-
logger.Log(LogLevel.Warning, 0, _state, ex, (s,e) => s);
802-
logger.Log(LogLevel.Information, 0, _state, ex, (s,e) => s);
803-
logger.Log(LogLevel.Debug, 0, _state, ex, (s,e) => s);
804-
logger.Log(LogLevel.Trace, 0, _state, ex, (s,e) => s);
799+
logger.Log(LogLevel.Critical, 0, _state, ex, (s, e) => s);
800+
logger.Log(LogLevel.Error, 0, _state, ex, (s, e) => s);
801+
logger.Log(LogLevel.Warning, 0, _state, ex, (s, e) => s);
802+
logger.Log(LogLevel.Information, 0, _state, ex, (s, e) => s);
803+
logger.Log(LogLevel.Debug, 0, _state, ex, (s, e) => s);
804+
logger.Log(LogLevel.Trace, 0, _state, ex, (s, e) => s);
805805

806806
// Assert
807807
Assert.Equal(12, sink.Writes.Count);
@@ -824,17 +824,29 @@ public void WriteCore_NullMessageWithNullException()
824824
string message = null;
825825

826826
// Act
827-
logger.Log(LogLevel.Critical, 0, message, ex, (s,e) => s);
828-
logger.Log(LogLevel.Error, 0, message, ex, (s,e) => s);
829-
logger.Log(LogLevel.Warning, 0, message, ex, (s,e) => s);
830-
logger.Log(LogLevel.Information, 0, message, ex, (s,e) => s);
831-
logger.Log(LogLevel.Debug, 0, message, ex, (s,e) => s);
832-
logger.Log(LogLevel.Trace, 0, message, ex, (s,e) => s);
827+
logger.Log(LogLevel.Critical, 0, message, ex, (s, e) => s);
828+
logger.Log(LogLevel.Error, 0, message, ex, (s, e) => s);
829+
logger.Log(LogLevel.Warning, 0, message, ex, (s, e) => s);
830+
logger.Log(LogLevel.Information, 0, message, ex, (s, e) => s);
831+
logger.Log(LogLevel.Debug, 0, message, ex, (s, e) => s);
832+
logger.Log(LogLevel.Trace, 0, message, ex, (s, e) => s);
833833

834834
// Assert
835835
Assert.Equal(0, sink.Writes.Count);
836836
}
837837

838+
[Fact]
839+
public void LogAfterDisposeDoesNotThrow()
840+
{
841+
var sink = new ConsoleSink();
842+
var console = new TestConsole(sink);
843+
var processor = new ConsoleLoggerProcessor();
844+
var logger = new ConsoleLogger(_loggerName, filter: null, includeScopes: false, loggerProcessor: processor);
845+
logger.Console = console;
846+
processor.Dispose();
847+
logger.LogInformation("Logging after dispose");
848+
}
849+
838850
private string GetMessage(string logLevelString, int eventId, Exception exception)
839851
=> GetMessage(logLevelString, eventId, _state, exception);
840852

@@ -849,9 +861,9 @@ private string GetMessage<TState>(string logLevelString, int eventId, TState sta
849861
+ _paddingString
850862
+ ReplaceMessageNewLinesWithPadding(state?.ToString())
851863
+ Environment.NewLine
852-
+ ( exception != null
864+
+ (exception != null
853865
? exception.ToString() + Environment.NewLine
854-
: string.Empty );
866+
: string.Empty);
855867
}
856868

857869
private string ReplaceMessageNewLinesWithPadding(string message)

0 commit comments

Comments
 (0)