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

    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)

Failed in #39376

Failed in #39082

Introduced by #38616

looking..

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

@ericstj

cc: @ericstj

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.

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

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.

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.

Can we make JIT deterministically fail here? Or would that hurt perf?

It would add a few extra instructions in steady state.

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

Either way should work.

For Microsoft.Extensions.Logging.Console.ConsoleLoggerProvider, I think it would be more appropriate to just ifdefed out the attribute. It is a big and slow library (e.g. using generic virtual methods). Micro-optimizations like this do not matter for it.

I see. I agree then that we need to be extra careful when using this. Should we use a naming convention to identify these PInvokes so it makes it clear at callsite when someone uses one of these methods?

Sounds reasonable. I cannot think about one that would look good. Do you have something in mind?

Some suffix on either interop class name or the method name (assuming this doesn’t increase overhead). Like Unsafe, Direct, Inline

Fix non-windows test failure on Microsoft.Extensions.Hosting.HostTest…

Navigation Menu

Repository navigation

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

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

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

Description

Activity

jkotas commented on Jul 16, 2020

jkotas commented on Jul 16, 2020

jkotas commented on Jul 16, 2020

maryamariyan commented on Jul 16, 2020

safern commented on Jul 16, 2020

safern commented on Jul 16, 2020

jkotas commented on Jul 16, 2020

safern commented on Jul 16, 2020

ericstj commented on Jul 16, 2020

jkotas commented on Jul 16, 2020

ericstj commented on Jul 16, 2020

jkotas commented on Jul 16, 2020

jkotas commented on Jul 16, 2020

ericstj commented on Jul 16, 2020

jkotas commented on Jul 16, 2020

ericstj commented on Jul 16, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions