Summary
LogOptions.CreateLogger(string) reads the LoggerFactory property (a Volatile.Read of s_loggerFactory) twice — once for the ReferenceEquals guard and once for the actual CreateLogger call. Because the backing field can be swapped concurrently (SetFactory and the property setter use Interlocked.Exchange), the guard and the create can observe different factory instances.
Location
LanguageTags/LogOptions.cs, in CreateLogger(string categoryName):
// LoggerFactory -> NullLogger
return !ReferenceEquals(LoggerFactory, NullLoggerFactory.Instance)
? LoggerFactory.CreateLogger(categoryName)
: NullLogger.Instance;
Problem
This is a check-then-act (TOCTOU) race. A thread can evaluate the guard against a real factory F1, then — if another thread calls SetFactory(...) in the window — create the logger from a different instance F2 (or from NullLoggerFactory if reset). It will not crash (NullLoggerFactory.CreateLogger is safe), but the decision and the action use inconsistent state, which defeats the thread-safe intent of the Volatile/Interlocked design.
Severity is low (worst case is a logger routed through the just-swapped factory during a narrow race window), but it is a trivial, worthwhile hardening for a library that advertises thread-safe logger configuration.
Suggested fix
Read the factory once into a local and use it for both the check and the create:
internal static ILogger CreateLogger(string categoryName)
{
ArgumentException.ThrowIfNullOrWhiteSpace(categoryName);
// Read the factory once so a concurrent swap cannot split the check
// and the create across two different instances.
ILoggerFactory factory = LoggerFactory;
return !ReferenceEquals(factory, NullLoggerFactory.Instance)
? factory.CreateLogger(categoryName)
: NullLogger.Instance;
}
The sibling ptr727.Utilities library adopts this same LogOptions and already applied this exact fix after a Copilot review flagged it (ptr727/Utilities#381). This issue is to keep the upstream in sync.
Minor / optional
The <remarks> XML doc block contains an em dash (...existing cached loggers—only new logger requests...). ptr727.Utilities replaced this with two ASCII sentences to satisfy its no-em-dash documentation rule. LanguageTags has no such rule, so this is purely an optional cross-repo consistency note, not a defect.
Summary
LogOptions.CreateLogger(string)reads theLoggerFactoryproperty (aVolatile.Readofs_loggerFactory) twice — once for theReferenceEqualsguard and once for the actualCreateLoggercall. Because the backing field can be swapped concurrently (SetFactoryand the property setter useInterlocked.Exchange), the guard and the create can observe different factory instances.Location
LanguageTags/LogOptions.cs, inCreateLogger(string categoryName):Problem
This is a check-then-act (TOCTOU) race. A thread can evaluate the guard against a real factory
F1, then — if another thread callsSetFactory(...)in the window — create the logger from a different instanceF2(or fromNullLoggerFactoryif reset). It will not crash (NullLoggerFactory.CreateLoggeris safe), but the decision and the action use inconsistent state, which defeats the thread-safe intent of theVolatile/Interlockeddesign.Severity is low (worst case is a logger routed through the just-swapped factory during a narrow race window), but it is a trivial, worthwhile hardening for a library that advertises thread-safe logger configuration.
Suggested fix
Read the factory once into a local and use it for both the check and the create:
The sibling
ptr727.Utilitieslibrary adopts this sameLogOptionsand already applied this exact fix after a Copilot review flagged it (ptr727/Utilities#381). This issue is to keep the upstream in sync.Minor / optional
The
<remarks>XML doc block contains an em dash (...existing cached loggers—only new logger requests...).ptr727.Utilitiesreplaced this with two ASCII sentences to satisfy its no-em-dash documentation rule. LanguageTags has no such rule, so this is purely an optional cross-repo consistency note, not a defect.