NullReferenceException thrown inside Metrics Library

[mcapodici]

I found an exception from the Metrics library written to our logs. I also have a fix which I will create a pull request for soon.

To reproduce the problem, create a new console application and add the v of Metrics.NET using NuGet. Paste the following Program.cs, enable exception debugging then start debugging from Visual Studio:

using Metrics;

namespace MetricsEx
{
	class Program
	{
		static void Main(string[] args)
		{
			Metric.Config.WithErrorHandler(x => { });
		}
	}
}

There are other ways to reproduce i.e. it is not specific to using WithErrorHandler.

The stack trace is:

 	Metrics.dll!Metrics.MetricsErrorHandler.MetricsErrorHandler() Line 11	C#
 	[Native to Managed Transition]	
 	[Managed to Native Transition]	
 	Metrics.dll!Metrics.MetricsConfig.MetricsConfig() Line 18	C#
 	[Native to Managed Transition]	
 	[Managed to Native Transition]	
>	MetricsEx.exe!MetricsEx.Program.Main(string[] args) Line 9	C#
 	[Native to Managed Transition]	
 	[Managed to Native Transition]	
 	mscorlib.dll!System.AppDomain.ExecuteAssembly(string assemblyFile, System.Security.Policy.Evidence assemblySecurity, string[] args)	Unknown
 	Microsoft.VisualStudio.HostingProcess.Utilities.dll!Microsoft.VisualStudio.HostingProcess.HostProc.RunUsersAssembly()	Unknown
 	mscorlib.dll!System.Threading.ThreadHelper.ThreadStart_Context(object state)	Unknown
 	mscorlib.dll!System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state, bool preserveSyncCtx)	Unknown
 	mscorlib.dll!System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state, bool preserveSyncCtx)	Unknown
 	mscorlib.dll!System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state)	Unknown
 	mscorlib.dll!System.Threading.ThreadHelper.ThreadStart()	Unknown

The cause is that in this line in the Metric class:

internal static readonly MetricsContext Internal { get; } = new DefaultMetricsContext("Metrics.NET");

The .NET framework surprisingly doesn't initialise the context object before the first read of the property.

The fix is to replace the property with a field:

internal static readonly MetricsContext Internal = new DefaultMetricsContext("Metrics.NET");

[SkyterX]

I couldn't reproduce the issue. Could you provide more info about targeting/installed .NET frameworks and compiler version on your machine? Also, a working solution with all proper settings would be very helpful.

The problem is very strange because the code generated by the compiler is equivalent to the suggested fix:

  public static class Metric
  {
    ...

    [CompilerGenerated]
    private static readonly MetricsContext <Internal>k__BackingField;

    internal static MetricsContext Internal
    {
      get
      {
        return Metric.<Internal>k__BackingField;
      }
    }

    static Metric()
    {
      Metric.log = LogProvider.GetCurrentClassLogger();
      Metric.<Internal>k__BackingField = (MetricsContext) new DefaultMetricsContext("Metrics.NET");
      ...
    }
    ...
  }

[mcapodici]

Target Framework: .NET Framework 4.5.2
Platform Target: Any CPU
Prefer 32-bit (checked)
Machine is 64-bit.

With the fix the compiled code (viewed with dotPeek) is different, you get just this:

internal static readonly MetricsContext Internal = new DefaultMetricsContext("Metrics.NET");

Looking at other issues on stack overflow e.g. http://stackoverflow.com/questions/7400438/why-this-static-constructor-is-not-getting-called, some of the answers display similar unexpected behaviour with respect to the order in which the the static constructor vs. accessing static members are called. It might be that an exception is thrown during Metric() and gets swallowed but this isn't caught in my debugger for some reason.

[SkyterX]

Ok, I think I've figured it out. The problem is not with the order of static fields and constructor initialization.
By default, dotPeek shows you sources based on .pdb files. If you look at 'Decompiled sources', you will see all the initialization moved to the static constructor:

    internal static readonly MetricsContext Internal;

    static Metric()
    {
      Metric.log = LogProvider.GetCurrentClassLogger();
      Metric.Internal = (MetricsContext) new DefaultMetricsContext("Metrics.NET");
      ...
    }

The difference in the current implementation is that the field Internal is accessed via property getter.
An exception occurs when this field is accessed by a static property in the MetricsErrorHandler class.
The weird thing is that the type initialization of MetricsErrorHandler happens before the initialization of Metric class. Two main problems are causing this:

1. The class MetricsErrorHandler doesn't have a static constructor, so the compiler marks it with beforefieldinit flag. This flag allows the type initializer be invoked at any time before the first reference to a static field of the class.
2. There is a circular dependency of static constructors between Metrics and MetricsErrorHandler. For example, when some exception is thrown while initializing fields of Metrics class, it's being caught and passes to MetricsErrorHandler, which have not been initialized yet.

The problem you are experiencing is caused by JIT compiler deciding to run the type initializer of MetricsErrorHandler before that of Metric. Adding an empty static constructor to the MetricsErrorHandler class appears to resolve the problem, but the second problem remains and needs some consideration #61 .

[bpowers]

I'm getting this as well, when following the instructions on the "Getting Started" wiki page (v0.5.1 from nuget):

using Metrics;

Metric.Config
    .WithHttpEndpoint("http://localhost:1234/")
    .WithAllCounters();

Is there any workaround for this that would let me run and evaluate the library while larger refactoring issues are considered?

[mcapodici]

@bpowers you could try merging in this fix I did earlier: mcapodici@22f347e. I don't think this will be the fix they will eventually use, but it may get you over the hump for now.

[PaulParau]

I didn't manage to reproduce the issue on my machine. Still, I have merged your request @mcapodici and I've added an empty static constructor to the MetricsErrorHandler class as @SkyterX suggested. You can find these changes in version 0.5.3-pre, I hope they fix the issues you were having @bpowers .

[PaulParau]

@bpowers have you by any chance checked whether this fixed the problems you were having? I'd like to close the issue if this is the case.