Skip to content

Commit

Permalink
Merge pull request serilog#733 from DmitryNaumov/remoting-issue
Browse files Browse the repository at this point in the history 
Prevent LogContext.Enrichers serialization when doing cross-domain calls
  • Loading branch information
@nblumhardt
nblumhardt authored Jun 19, 2016
2 parents 4d15d2c + 3b55d2d commit 81ba6b3
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 145 deletions.
20 changes: 0 additions & 20 deletions src/Serilog/Context/ImmutableStack.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,38 +14,18 @@

using System;
using System.Collections.Generic;
#if REMOTING
using System.Runtime.Serialization;
#endif

// General-purpose type; not all features are used here.
// ReSharper disable MemberCanBePrivate.Global
// ReSharper disable MemberCanBeProtected.Global

namespace Serilog.Context
{
// Needed because of the shallow-copying behaviour of
// LogicalCallContext.
#if REMOTING
[Serializable]
class ImmutableStack<T> : IEnumerable<T>, ISerializable
#else
class ImmutableStack<T> : IEnumerable<T>
#endif
{
readonly ImmutableStack<T> _under;
readonly T _top;

#if REMOTING
public ImmutableStack(SerializationInfo info, StreamingContext context)
{
}

void ISerializable.GetObjectData(SerializationInfo info, StreamingContext context)
{
}
#endif

ImmutableStack()
{
}
Expand Down
64 changes: 6 additions & 58 deletions src/Serilog/Context/LogContext.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
using System.Collections.Generic;
using System.Threading;
#elif REMOTING
using System.Runtime.Remoting;
using System.Runtime.Remoting.Messaging;
#endif

Expand Down Expand Up @@ -53,7 +54,7 @@ public static class LogContext
#if ASYNCLOCAL
static readonly AsyncLocal<ImmutableStack<ILogEventEnricher>> Data = new AsyncLocal<ImmutableStack<ILogEventEnricher>>();
#elif REMOTING
static readonly string DataSlotName = typeof(LogContext).FullName;
static readonly string DataSlotName = typeof(LogContext).FullName + "@AppDomain" + AppDomain.CurrentDomain.Id;
#else // DOTNET_51
[ThreadStatic]
static ImmutableStack<ILogEventEnricher> Data;
Expand Down Expand Up @@ -105,25 +106,6 @@ public static IDisposable PushProperties(params ILogEventEnricher[] properties)
return bookmark;
}

/// <summary>
/// Remove all data from the context so that
/// cross-app-domain calls can be made without requiring
/// Serilog assemblies to be present in the remote domain.
/// </summary>
/// <returns>A token that will restore the suspended log context data, if any.</returns>
/// <remarks>The <see cref="LogContext"/> should not be manipulated further
/// until the return value from this method has been disposed.</remarks>
/// <returns></returns>
public static IDisposable Suspend()
{
var stack = GetOrCreateEnricherStack();
var bookmark = new ContextStackBookmark(stack);

Enrichers = null;

return bookmark;
}

static ImmutableStack<ILogEventEnricher> GetOrCreateEnricherStack()
{
var enrichers = Enrichers;
Expand Down Expand Up @@ -178,51 +160,17 @@ static ImmutableStack<ILogEventEnricher> Enrichers

#elif REMOTING

/// <summary>
/// When calling into appdomains without Serilog loaded, e.g. via remoting or during unit testing,
/// it may be necesary to set this value to true so that serialization exceptions are avoided. When possible,
/// using the <see cref="Suspend"/> method in a using block around the call has a lower overhead and
/// should be preferred.
/// </summary>
// ReSharper disable once UnusedAutoPropertyAccessor.Global
public static bool PermitCrossAppDomainCalls { get; set; }

sealed class Wrapper : MarshalByRefObject
{
public ImmutableStack<ILogEventEnricher> Value { get; set; }
}

static object GetContext(ImmutableStack<ILogEventEnricher> value)
{
var context = !PermitCrossAppDomainCalls ? (object)value : new Wrapper
{
Value = value
};
return context;
}

static ImmutableStack<ILogEventEnricher> Enrichers
{
get
{
var data = CallContext.LogicalGetData(DataSlotName);

ImmutableStack<ILogEventEnricher> context;
if (PermitCrossAppDomainCalls)
{
context = ((Wrapper)data)?.Value;
}
else
{
context = (ImmutableStack<ILogEventEnricher>)data;
}

return context;
var objectHandle = CallContext.LogicalGetData(DataSlotName) as ObjectHandle;

return objectHandle?.Unwrap() as ImmutableStack<ILogEventEnricher>;
}
set
{
var context = GetContext(value);
CallContext.LogicalSetData(DataSlotName, context);
CallContext.LogicalSetData(DataSlotName, new ObjectHandle(value));
}
}

Expand Down
76 changes: 9 additions & 67 deletions test/Serilog.Tests/Context/LogContextTests.cs
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
using System;
using System.IO;
using Xunit;
using Xunit;
using Serilog.Context;
using Serilog.Events;
using Serilog.Core.Enrichers;
using Serilog.Tests.Support;
#if REMOTING
using System;
using System.IO;
using System.Runtime.Remoting.Messaging;
#endif
using System.Threading;
Expand All @@ -18,7 +18,6 @@ public class LogContextTests
public LogContextTests()
{
#if REMOTING
LogContext.PermitCrossAppDomainCalls = false;
CallContext.LogicalSetData(typeof(LogContext).FullName, null);
#endif
}
Expand Down Expand Up @@ -112,43 +111,10 @@ public async Task ContextPropertiesCrossAsyncCalls()
}
}

#if REMOTING
[Fact]
public async Task ContextPropertiesPersistWhenCrossAppDomainCallsAreEnabled()
{
LogEvent lastEvent = null;

var log = new LoggerConfiguration()
.Enrich.FromLogContext()
.WriteTo.Sink(new DelegatingSink(e => lastEvent = e))
.CreateLogger();

LogContext.PermitCrossAppDomainCalls = true;

using (LogContext.PushProperty("A", 1))
{
var pre = Thread.CurrentThread.ManagedThreadId;

await Task.Delay(1000);

var post = Thread.CurrentThread.ManagedThreadId;

log.Write(Some.InformationEvent());
Assert.Equal(1, lastEvent.Properties["A"].LiteralValue());

// No problem if this happens occasionally; was Assert.Inconclusive().
// The test was marshalled back to the same thread after awaiting.
Assert.NotSame(pre, post);
}
}
#endif

#if APPDOMAIN
// Must not actually try to pass context across domains,
// since user property types may not be serializable.
// Fails if the Serilog assemblies cannot be loaded in the
// remote domain. See also LogContext.Suspend()
[Fact(Skip="Needs to be updated for dotnet runner.")]
[Fact(Skip = "Needs to be updated for dotnet runner.")]
public void DoesNotPreventCrossDomainCalls()
{
var projectRoot = Environment.CurrentDirectory;
Expand All @@ -169,8 +135,8 @@ public void DoesNotPreventCrossDomainCalls()

var domaininfo = new AppDomainSetup
{
ApplicationBase = Path.Combine(projectRoot, @"artifacts\"),
PrivateBinPath = @"src\Serilog\bin\Debug\net45;test\Serilog.Tests\bin\Debug\net452".Replace("Debug", configuration)
ApplicationBase = projectRoot,
PrivateBinPath = @"test\Serilog.Tests\bin\Debug\net452\win7-x64".Replace("Debug", configuration)
};
var evidence = AppDomain.CurrentDomain.Evidence;
domain = AppDomain.CreateDomain("LogContextTest", evidence, domaininfo);
Expand All @@ -187,48 +153,24 @@ public void DoesNotPreventCrossDomainCalls()
}
}
#endif

[Fact]
public void WhenSuspendedAllPropertiesAreRemovedFromTheContext()
{
LogEvent lastEvent = null;

var log = new LoggerConfiguration()
.Enrich.FromLogContext()
.WriteTo.Sink(new DelegatingSink(e => lastEvent = e))
.CreateLogger();

using (LogContext.PushProperty("A1", 1))
{
using (LogContext.Suspend())
{
log.Write(Some.InformationEvent());
Assert.False(lastEvent.Properties.ContainsKey("A1"));
}

log.Write(Some.InformationEvent());
Assert.Equal(1, lastEvent.Properties["A1"].LiteralValue());
}
}
}

#if REMOTING
public class RemotelyCallable : MarshalByRefObject
{
public bool IsCallable()
{
var sw = new StringSink(outputTemplate: "{Anything}{Number}");
LogEvent lastEvent = null;

var log = new LoggerConfiguration()
.WriteTo.Sink(sw)
.WriteTo.Sink(new DelegatingSink(e => lastEvent = e))
.Enrich.FromLogContext()
.CreateLogger();

using (LogContext.PushProperty("Number", 42))
log.Information("Hello");

var s = sw.ToString();
return s == "42";
return 42.Equals(lastEvent.Properties["Number"].LiteralValue());
}
}
#endif
Expand Down

0 comments on commit 81ba6b3

Please sign in to comment.