make log4net usable from a PublishAOT build (#306) - #306
Conversation
Native AOT broke two things in the startup path (fixes #233 partially). Assembly.GetCallingAssembly() throws PlatformNotSupportedException there, so every overload that resolves the repository from the caller failed - LogManager.GetLogger(Type) among them. Guard the 18 call sites with CallerAssembly.IsSupported, a flag probed once, and fall back to the entry assembly when the runtime does not implement the call. The call itself has to stay in the public method whose caller is wanted, so it cannot be moved into the helper. SystemInfo.GetAppSetting() then failed as well, because System.Configuration is trimmed away. Its catch never saw that: resolving the missing assembly fails on entry to the method, before the try region, so the exception escaped the static constructor as a TypeInitializationException and killed the process. Read the setting through a separate, never inlined method so the failure is raised inside the try block, latch the result so a permanent failure is reported once rather than per lookup, and fall back to environment variables the way the Android branch already does. That fallback also makes log4net.NullText and log4net.NotAvailableText settable under AOT, where they previously could not be configured at all. Note that the fallback applies on .NET Framework too: a malformed app.config now reads settings from the environment instead of returning null. This does not make log4net AOT-clean - repositories, appenders and layouts are still instantiated via Activator.CreateInstance, so an AOT app still fails with MissingMethodException on Hierarchy's constructor.
26841b2 to
9ca8d88
Compare
Native AOT broke log4net in three ways (#233). Assembly.GetCallingAssembly() throws PlatformNotSupportedException there, so every overload that resolves the repository from the caller failed - LogManager.GetLogger(Type) among them. Guard the 18 call sites with CallerAssembly.IsSupported, a flag probed once, and fall back to the entry assembly when the runtime does not implement the call. The call itself has to stay in the public method whose caller is wanted, so it cannot be moved into the helper. SystemInfo.GetAppSetting() then reported a caught failure on every lookup, because a trimmed System.Configuration cannot initialize. Tell that apart from a configuration file that does not parse - Native AOT surfaces both as a ConfigurationErrorsException, so only the inner exception distinguishes them - and treat a missing configuration system as a property of the runtime rather than a fault: log it at debug level and let environment variables stand in for the config file, as they already do on Android. A malformed config file is still reported as an error and still yields no setting. Finally the trimmer removed the constructors of everything log4net creates reflectively, so no repository, pattern converter or locking model could be instantiated. Annotate that flow with DynamicallyAccessedMembers - polyfilled here, because the trimmer matches it by name and neither target framework declares it - and hold the built-in converters in a Dictionary of ConverterInfo rather than of Type, since a Type placed in a collection loses its annotation. The registries are now built through a generic method whose new() constraint states the same requirement structurally, so a converter without a public parameterless constructor fails to compile instead of failing in a trimmed build. Configuration still has to be done in code: XmlConfigurator names its types in strings and cannot work once they have been trimmed. Document that, and the fact that loggers from non-entry assemblies land in the entry assembly's repository, on a new Native AOT page in the manual.
9ca8d88 to
bf839f7
Compare
fluffynuts
left a comment
There was a problem hiding this comment.
nice work - just a suggestion to consolidate the logic which determines the caller assembly or falls back on the entry assembly.
| /// <seealso cref="Log4NetConfigurationSectionHandler"/> | ||
| public static ICollection Configure() | ||
| => Configure(LogManager.GetRepository(Assembly.GetCallingAssembly())); | ||
| => Configure(LogManager.GetRepository(CallerAssembly.IsSupported ? Assembly.GetCallingAssembly() : CallerAssembly.Fallback)); |
There was a problem hiding this comment.
I see this logic quite a few times throughout - perhaps move to CallerAssembly with a lazy backing field and reference that static property elsewhere (eg CallerAssembly.ResolvedCallerAssembly? Main reason being that it's no longer just an obvious call to Assembly.GetCallingAssembly(), but now includes logic, which is repeated in quite a few places.
There was a problem hiding this comment.
@fluffynuts
Good catch, but this one can't move - though I don't like it either. Assembly.GetCallingAssembly()
returns the caller of the method containing the call, so in a property on CallerAssembly the caller
is log4net itself - every logger would land in log4net's own repository. Two-assembly harness,
called from UserApp:
inline (current PR) -> UserApp <- correct
via property (suggested) -> log4net
The lazy backing field is worse: the first assembly to touch it wins forever, so the result depends on
load order.
The BCL hits this exact problem and needs an internal enum for it - System.Threading.StackCrawlMark
(LookForMyCaller, LookForMyCallersCaller), passed by ref so Assembly.Load can delegate to a
private helper. It's NotPublic and no public API accepts it. It also wouldn't help us: it's
stack-walking machinery, and AOT throws precisely because there is no stack to walk.
Only a Roslyn interceptor would actually remove the repetition - left out here, but I'm happy to open a separate issue.
There was a problem hiding this comment.
Personally, I think [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]
is the bigger eyesore.
Native AOT broke log4net in three ways (#233).
Assembly.GetCallingAssembly() throws PlatformNotSupportedException there, so
every overload that resolves the repository from the caller failed -
LogManager.GetLogger(Type) among them. Guard the 18 call sites with
CallerAssembly.IsSupported, a flag probed once, and fall back to the entry
assembly when the runtime does not implement the call. The call itself has to
stay in the public method whose caller is wanted, so it cannot be moved into
the helper.
SystemInfo.GetAppSetting() then reported a caught failure on every lookup,
because a trimmed System.Configuration cannot initialize. Tell that apart from
a configuration file that does not parse - Native AOT surfaces both as a
ConfigurationErrorsException, so only the inner exception distinguishes them -
and treat a missing configuration system as a property of the runtime rather
than a fault: log it at debug level and let environment variables stand in for
the config file, as they already do on Android. A malformed config file is
still reported as an error and still yields no setting.
Finally the trimmer removed the constructors of everything log4net creates
reflectively, so no repository, pattern converter or locking model could be
instantiated. Annotate that flow with DynamicallyAccessedMembers - polyfilled
here, because the trimmer matches it by name and neither target framework
declares it - and hold the built-in converters in a Dictionary of ConverterInfo
rather than of Type, since a Type placed in a collection loses its annotation.
The registries are now built through a generic method whose new() constraint
states the same requirement structurally, so a converter without a public
parameterless constructor fails to compile instead of failing in a trimmed
build.
Configuration still has to be done in code: XmlConfigurator names its types in
strings and cannot work once they have been trimmed. Document that, and the
fact that loggers from non-entry assemblies land in the entry assembly's
repository, on a new Native AOT page in the manual.