Repository navigation
stop stale context reaching logical stacks and fixed events - #323
Merged
Merged
Conversation
FreeAndNil
added a commit
that referenced
this pull request
Sep 21, 2026
- context value into a PatternString-typed config value: the operator's choice - trustworthiness of that value: the deployer's responsibility - comment at the converter, note in our FAQ, where the next reader looks audit da18b6f-f021, no code change.
FreeAndNil
added a commit
that referenced
this pull request
Sep 21, 2026
- writes to HttpContext.Current of the appending thread - behind a buffering appender a flush hits the triggering request - a timestamp filter would drop foreign events, so documented not enforced audit da18b6f-f006, no code change.
FreeAndNil
added a commit
that referenced
this pull request
Sep 21, 2026
- Dispose rebuilt the stack from the copy taken at Push time - after Clear, or after a Pop below its depth, the earlier frames came back - now trims the stack registered in the flow, never grows it - the stack holds its owning stacks instead of a register callback - residual: a foreign frame trims the current flow, loss not injection 1M ops, best of 5, Release on net10.0: | case | before | after | |----------------------|-------------------------|-------------------------| | push/dispose depth 1 | 405 to 438 ns, 1272 B | 412 to 458 ns, 1272 B | | push/dispose depth 3 | 1386 to 1433 ns, 3976 B | 1414 to 1440 ns, 3976 B | | dispose after Clear | 548 ns, 1656 B | 361 ns, 1016 B | audit da18b6f-f044.
FreeAndNil
added a commit
that referenced
this pull request
Sep 21, 2026
- existed for the logical stack's register callback, now an owner reference - unused in the tree, public so it stays for now - marked obsolete, removal in version 4
FreeAndNil
force-pushed
the
Feature/323-event-and-context-state
branch
from
September 21, 2026 18:21
e123b49 to
4ef6dea
Compare
FreeAndNil
added a commit
that referenced
this pull request
Sep 21, 2026
- the cache was unlocked before the fields to fix were determined - a redundant Fix reopened it with nothing to do - a reader then cached its own identity, thread name or location - measured: a hit after 56000 to 63000 calls, five runs of five audit da18b6f-f039.
- context value into a PatternString-typed config value: the operator's choice - trustworthiness of that value: the deployer's responsibility - comment at the converter, note in our FAQ, where the next reader looks audit da18b6f-f021, no code change.
- writes to HttpContext.Current of the appending thread - behind a buffering appender a flush hits the triggering request - a timestamp filter would drop foreign events, so documented not enforced audit da18b6f-f006, no code change.
- Dispose rebuilt the stack from the copy taken at Push time - after Clear, or after a Pop below its depth, the earlier frames came back - now trims the stack registered in the flow, never grows it - the stack holds its owning stacks instead of a register callback - residual: a foreign frame trims the current flow, loss not injection 1M ops, best of 5, Release on net10.0: | case | before | after | |----------------------|-------------------------|-------------------------| | push/dispose depth 1 | 405 to 438 ns, 1272 B | 412 to 458 ns, 1272 B | | push/dispose depth 3 | 1386 to 1433 ns, 3976 B | 1414 to 1440 ns, 3976 B | | dispose after Clear | 548 ns, 1656 B | 361 ns, 1016 B | audit da18b6f-f044.
- existed for the logical stack's register callback, now an owner reference - unused in the tree, public so it stays for now - marked obsolete, removal in version 4
- the cache was unlocked before the fields to fix were determined - a redundant Fix reopened it with nothing to do - a reader then cached its own identity, thread name or location - measured: a hit after 56000 to 63000 calls, five runs of five audit da18b6f-f039.
FreeAndNil
force-pushed
the
Feature/323-event-and-context-state
branch
from
September 22, 2026 19:05
4ef6dea to
e725966
Compare
FreeAndNil
marked this pull request as ready for review
September 22, 2026 19:07
fluffynuts
approved these changes
Sep 23, 2026
gdziadkiewicz
approved these changes
Sep 23, 2026
Contributor
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
Review effort: Lite
Findings: 2
Open (2)
What changed in this PR
Fixes two concurrency/context bugs in log4net: (1) disposed LogicalThreadContext.Stacks frames resurrecting removed frames, and (2) redundant LoggingEvent.Fix reopening the event cache and allowing cross-thread contamination.
Changes:
- Rework
LogicalThreadContextStackownership/registration soDisposetrims the currently-registered stack rather than restoring a stale copy, plus new regression tests. - Change
LoggingEvent.FixVolatileDatato only unlock the cache when there is actual work to do and make_cacheUpdatablevolatile, plus a new multithreaded regression test. - Documentation/changelog updates for
%propertypath traversal considerations,AspNetTraceAppenderconstraints, and deprecation ofTwoArgAction.
| File | Description |
|---|---|
| src/site/antora/modules/ROOT/pages/manual/faq.adoc | Adds security note about %property expansion and path traversal risk. |
| src/site/antora/modules/ROOT/pages/manual/configuration/appenders.adoc | Documents AspNetTraceAppender request/thread affinity constraint. |
| src/log4net/Util/PatternStringConverters/PropertyPatternConverter.cs | Adds in-code comment referencing path-traversal guidance for %property. |
| src/log4net/Util/LogicalThreadContextStacks.cs | Changes stack creation to pass owner; adds owner APIs for current-stack lookup/registration. |
| src/log4net/Util/LogicalThreadContextStack.cs | Replaces callback delegate with owner reference; fixes Dispose to trim current flow. |
| src/log4net/Core/LoggingEvent.cs | Prevents redundant Fix from reopening cache; makes cache flag volatile. |
| src/log4net/Appender/AspNetTraceAppender.cs | Adds remarks warning not to wrap in buffering appenders. |
| src/log4net.Tests/Core/FixingTest.cs | Adds regression test for cross-thread identity caching via redundant Fix. |
| src/log4net.Tests/Context/LogicalThreadContextTest.cs | Adds regression tests for stack dispose not resurrecting frames across clear/pop/flows. |
| src/changelog/3.5.0/323-logical-stack-dispose.xml | Changelog entry for logical stack dispose fix. |
| src/changelog/3.5.0/323-logging-event-fix-cache.xml | Changelog entry for logging-event cache reopening fix. |
| src/changelog/3.5.0/323-deprecate-twoargaction.xml | Changelog entry for TwoArgAction deprecation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This was referenced Oct 1, 2026
This was referenced Oct 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixed
Disposeused the copy of the stack made atPushtimeClear, or after twoPopcalls, the old frames came backnothing is added
Fixopened the event cache again, audit da18b6f-f039Deprecated
log4net.Util.TwoArgAction, the old callback of the stack, now unused, to be removed in v4Documented, not changed
%propertyin aPatternString-typed setting, audit da18b6f-f021traversal
AspNetTraceAppenderbelongs to one request, audit da18b6f-f006Speed of the stack change
1M calls, best of 5, Release on net10.0:
The normal cases are the same within noise and use the same memory. The fixed case is 34 percent
faster.