Microsoft.Extensions.Logging.Console.ConsoleLoggerProvider tests failing on non-Windows platforms

[jkotas]

    Microsoft.Extensions.Hosting.HostTests.CreateDefaultBuilder_RegistersEventSourceLogger [FAIL]
      System.DllNotFoundException : Unable to load shared library 'kernel32.dll' or one of its dependencies. In order to help diagnose loading problems, consider setting the LD_DEBUG environment variable: libkernel32.dll: cannot open shared object file: No such file or directory
      Stack Trace:
           at Microsoft.Extensions.Logging.Console.ConsoleLoggerProvider.DoesConsoleSupportAnsi()
        /_/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProvider.cs(50,0): at Microsoft.Extensions.Logging.Console.ConsoleLoggerProvider..ctor(IOptionsMonitor`1 options, IEnumerable`1 formatters)
           at System.RuntimeMethodHandle.InvokeMethod(Object target, Object[] arguments, Signature sig, Boolean constructor, Boolean wrapExceptions)
        /_/src/coreclr/src/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs(348,0): at System.Reflection.RuntimeConstructorInfo.Invoke(BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)

Tagging subscribers to this area: @eiriktsarpalis
Notify danmosemsft if you want to be subscribed.

[jkotas]

Failed in #39376

[jkotas]

Failed in #39082

[jkotas]

Introduced by #38616

[maryamariyan]

looking..

[safern]

Introduced by #38616

I'm investigating the failure and I believe this is because of: #39209

kernel32.dll shouldn't come into play in OSX nor Linux because of this check: https://github.com/dotnet/runtime/blob/master/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProvider.cs#L64 -- and Linux and OSX test runs are failing with:

Microsoft.Extensions.Hosting.HostTests.CreateDefaultBuilder_IncludesContentRootByDefault [FAIL]
      System.DllNotFoundException : Unable to load shared library 'kernel32.dll' or one of its dependencies. In order to help diagnose loading problems, consider setting the DYLD_PRINT_LIBRARIES environment variable: dlopen(libkernel32.dll, 1): image not found
      Stack Trace:
           at Microsoft.Extensions.Logging.Console.ConsoleLoggerProvider.DoesConsoleSupportAnsi()
        /_/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProvider.cs(50,0): at Microsoft.Extensions.Logging.Console.ConsoleLoggerProvider..ctor(IOptionsMonitor`1 options, IEnumerable`1 formatters)

I'm doing a run without #39209

[safern]

cc: @ericstj

[jkotas]

I do not think that #39209 is the problem. I believe that the problem is how this is calling P/Invokes with SuppressGCTransitionAttribute.

One of the side-effects of SuppressGCTransitionAttribute is that the JIT will load the target eagerly to minimize overhead. It means that it does not compose well with platform specific checks in platform neutral assemblies.

I think this assembly should be either made platform specific (without the IsOSPlatform checks), or the use of SuppressGCTransitionAttribute should be ifdefed out in this assembly.

[safern]

I see... I think reviewers were pushing against making it platform specific to avoid much overhead in the package.

[ericstj]

What’s better, ifdef out the attribute or make a non-inlinable method?

Are we OK with the attribute behaving this way? In the past we haven’t needed to think about inlining and pinvokes.

[jkotas]

What’s better, ifdef out the attribute or make a non-inlinable method?

Either way should work.

This attribute is for advanced cases where one needs to reduce PInvoke overhead to minimum around a cheap PInvoke. If you are using it, you have to think about its interaction with inlining.