Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed null-reference-exception when parsing empty scope-property collection #233

Merged
merged 2 commits into from
Aug 3, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions src/NLog.Extensions.Logging/Logging/NLogBeginScopeParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -99,31 +99,31 @@ public static ScopeProperties CaptureScopeProperties(IReadOnlyList<KeyValuePair<

public static ScopeProperties CaptureScopeProperties(System.Collections.IEnumerable scopePropertyCollection, ConcurrentDictionary<Type, KeyValuePair<Func<object, object>, Func<object, object>>> stateExractor)
{
ScopeProperties scope = null;
ScopeProperties scope = new ScopeProperties();

var keyValueExtractor = default(KeyValuePair<Func<object, object>, Func<object, object>>);
foreach (var property in scopePropertyCollection)
{
if (property == null)
return null;
break;

if (scope == null)
if (keyValueExtractor.Key == null)
{
if (!TryLookupExtractor(stateExractor, property.GetType(), out keyValueExtractor))
return null;

scope = new ScopeProperties();
break;
}

AddKeyValueProperty(scope, keyValueExtractor, property);
}

scope.AddDispose(CreateDiagnosticLogicalContext(scopePropertyCollection));
return scope;
}

public static ScopeProperties CaptureScopeProperty<TState>(TState scopeProperty, ConcurrentDictionary<Type, KeyValuePair<Func<object, object>, Func<object, object>>> stateExractor)
public static IDisposable CaptureScopeProperty<TState>(TState scopeProperty, ConcurrentDictionary<Type, KeyValuePair<Func<object, object>, Func<object, object>>> stateExractor)
{
if (!TryLookupExtractor(stateExractor, scopeProperty.GetType(), out var keyValueExtractor))
return null;
return CreateDiagnosticLogicalContext(scopeProperty);

var scope = new ScopeProperties();
AddKeyValueProperty(scope, keyValueExtractor, scopeProperty);
Expand Down
1 change: 1 addition & 0 deletions src/NLog.Extensions.Logging/Logging/NLogLogger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,7 @@ private NullScope()
/// <inheritdoc />
public void Dispose()
{
// Nothing to do
}
}
}
Expand Down
19 changes: 13 additions & 6 deletions src/NLog.Extensions.Logging/Logging/NLogLoggerFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,25 @@ public NLogLoggerFactory(NLogProviderOptions options)
_provider = new NLogLoggerProvider(options);
}


#region Implementation of IDisposable

/// <summary>
/// Dispose
/// Cleanup
/// </summary>
public void Dispose()
{
LogManager.Flush();
Dispose(true);
GC.SuppressFinalize(this);
}

#endregion
/// <summary>
/// Cleanup
/// </summary>
protected virtual void Dispose(bool disposing)
{
if (disposing)
{
LogManager.Flush();
}
}

#region Implementation of ILoggerFactory

Expand Down
15 changes: 14 additions & 1 deletion src/NLog.Extensions.Logging/Logging/NLogLoggerProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,20 @@ public Microsoft.Extensions.Logging.ILogger CreateLogger(string name)
/// </summary>
public void Dispose()
{
LogManager.Flush();
Dispose(true);
GC.SuppressFinalize(this);
}

/// <summary>
/// Cleanup
/// </summary>
/// <param name="disposing"></param>
protected virtual void Dispose(bool disposing)
{
if (disposing)
{
LogManager.Flush();
}
}

/// <summary>
Expand Down
22 changes: 21 additions & 1 deletion test/CustomBeginScopeTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public void TestNonSerializableSayHello()
}

[Fact]
public void TestNonSerializableSayNothing()
public void TestNonSerializableSayHelloWithScope()
{
var runner = GetRunner<CustomBeginScopeTestRunner>(new NLogProviderOptions() { IncludeScopes = false });
var target = new NLog.Targets.MemoryTarget() { Layout = "${message} ${mdlc:World}. Welcome ${ndlc}" };
Expand All @@ -43,6 +43,17 @@ public void TestNonSerializableSayHi()
Assert.Equal("Earth People", scopeString);
}

[Fact]
public void TestNonSerializableSayNothing()
{
var runner = GetRunner<CustomBeginScopeTestRunner>();
var target = new NLog.Targets.MemoryTarget() { Layout = "${message}" };
NLog.Config.SimpleConfigurator.ConfigureForTargetLogging(target);
runner.SayNothing().Wait();
Assert.Single(target.Logs);
Assert.Equal("Nothing", target.Logs[0]);
}

public class CustomBeginScopeTestRunner
{
private readonly ILogger<CustomBeginScopeTestRunner> _logger;
Expand Down Expand Up @@ -70,6 +81,15 @@ public async Task<string> SayHi()
return scopeState.ToString();
}
}

public async Task SayNothing()
{
using (var scopeState = _logger.BeginScope(new Dictionary<string,string>()))
{
await Task.Yield();
_logger.LogInformation("Nothing");
}
}
}

private class ActionLogScope : IReadOnlyList<KeyValuePair<string, object>>
Expand Down