Repository navigation
Make the encoder stack-depth tests hold on a reused thread stack - #4722
Conversation
DecodeWithUnboundedNestingLimitFailsBeforeTheStackOverflows(Json, VariantArray) and (Json, DataValue) failed intermittently on Linux CI: the 450-level JSON input decoded without tripping the stack check. A thread that asks for a 256 KB stack can run on more. glibc reuses the cached stack of an exited thread when it is at most four times the requested size, and this fixture's own 1 MB threads leave such stacks behind. The stack check measures the stack the thread got, and 450 JSON levels fit a 1 MB stack. Running a 1 MB thread right before the decode reproduces the failure on Linux 20 times out of 20; Windows does not reuse stacks this way. The unbounded-limit tests now run on the 1 MB and the 256 KB stack and nest 5000 levels, twice the deepest nesting of any codec and shape that fits a 1 MB stack (2477 levels: JSON EncodeableTree, Linux, Release). The 2000 levels of the other codecs were too few as well: on Linux, encoding a Binary EncodeableTree of 2000 levels fits a 1 MB stack. The JSON input is written by hand because the JsonEncoder cannot nest deeper than 1000 levels, and NestedJsonMatchesTheJsonEncoder keeps it equal to the encoder's output. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟢 Approval recommended
The focused test changes correctly address reused thread stacks and validate the handcrafted JSON inputs.
0 open findings
What changed in this PR
Stabilizes encoder stack-depth tests against Linux thread-stack reuse behavior.
Changes:
- Tests both 256 KB and 1 MB thread stacks at 5,000 nesting levels.
- Generates deeply nested JSON manually and validates it against
JsonEncoderoutput.
| File | Description |
|---|---|
tests/Opc.Ua.Types.Tests/Encoders/EncoderStackDepthTests.cs |
Strengthens stack-overflow regression coverage across codecs and shapes. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Code coverage✅ Coverage gate passed.
Coverage is above the recorded baseline - consider ratcheting Thresholds live in |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4722 +/- ##
==========================================
- Coverage 82.84% 82.83% -0.02%
==========================================
Files 2267 2267
Lines 345943 345943
Branches 60303 60303
==========================================
- Hits 286609 286568 -41
- Misses 39238 39269 +31
- Partials 20096 20106 +10
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Description
EncoderStackDepthTests.DecodeWithUnboundedNestingLimitFailsBeforeTheStackOverflows(Json, VariantArray)and(Json, DataValue)fail intermittently on Linux CI, for example in run 37921301898, attempt 1,test-linux-net10.0 (12/15). The 450-level JSON input decodes without tripping the stack check, so the expectedBadEncodingLimitsExceededis missing. A re-run passes.Cause: a thread that asks for a 256 KB stack can run on more. glibc reuses the cached stack of an exited thread when it is at most four times the requested size, and the fixture's own 1 MB threads leave such stacks behind.
RuntimeHelpers.EnsureSufficientExecutionStackmeasures the stack the thread actually got, and 450 JSON levels fit a 1 MB stack. The product's stack check is correct; the test relied on the requested size.Deepest nesting that fits before the check fires (net10.0, Release):
Fix:
NestedJsonMatchesTheJsonEncoderchecks it against the encoder's output for every shape.Validation
mcr.microsoft.com/dotnet/sdk:10.0container, each decode right after a 1 MB thread exits, 20 times per shapedotnet test tests/Opc.Ua.Types.Tests -f net10.0 --filter FullyQualifiedName~EncoderStackDepthTests, Release and Debug on Windows, 2 runs each-f net48EncodeWithUnboundedNestingLimitFailsBeforeTheStackOverflows(Binary,EncodeableTree)fails on the 1 MB stackEncodingLimits.EnsureSufficientStacktemporarily disabledRelated Issues
Checklist