Stack memory issue in locale tests (overflows on Mac)
- Dominant language
- C#
- Stars
- 9.7k
- Forks
- 538
- PR merge metrics
- No merged PRs in 30d
Description
### Bogus NuGet Package
35.5.0
### .NET Version
.NET 8
### Visual Studio Version
VS Code
### What operating system are you using?
MacOS
### What locale are you using with Bogus?
en
### Problem Description
On MacOS I get a stack overflow in the `LocaleSchemaTests.ensure_wellknown_locale_schema()` test for the `en` locale.
After a bit of investigation it seems to be related to the custom `InterceptedContractResolver`'s `ResolveContract()` method, as I think we're (inadvertently, due to an Argon implementation detail) reassigning the `InterceptSerializeItem` callback such that it's essentially 'repeating' **itself and ends up running O(n^2) times** relative to the size of the JSON.
I think it's happening with the en locale because it's the largest.
And I think it's only on Mac because there's probably less stack memory available ([512KB for secondary threads apparently](https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/Multithreading/CreatingThreads/CreatingThreads.html#//apple_ref/doc/uid/10000057i-CH15-SW2)).
### LINQPad Example or Reproduction Steps
## Steps to reproduce stack overflow on Mac
* Check out repo
* build using `dotnet nuke --root .` (I think this is the equivalent of the `build.cmd` script)
* run tests using `dotnet nuke test --root .` or just use `dotnet test` or VS Code's test explorer
## Gist to demonstrate memory issue
We can show the issue using a copy of the Resolver, with a trivial JSON and the help of some (admittedly crude) Console.WriteLine debugging.
https://gist.github.com/dangerman/b3d674f4628dc9e5225f0618e1459215
In this example, we have a JSON full of fruit, and the corresponding one is printed whenever `InterceptSerializeItem` is run.
| Expected | Actual |
| -- | -- |
| |
|
### Expected Behavior
Tests run and pass
### Actual Behavior
Stack overflow
...
### Cause
We're using a custom `InterceptedContractResolver` to simplify arrays in our Verify Snapshots (so that arrays get summarised like `Numbers [string, 20]`).
In the `ResolveContract()` implementation we're:
* saving the original `jdc.InterceptSerializeItem` as `defaultIntercept`
* assigning `jdc.InterceptSerializeItem = (key, val)...`
* and in this callback, we either summarise the list, or we call the `defaultIntercept()` as necessary
https://github.com/bchavez/Bogus/blob/b72a4f9de627363b18b45fbdc7ff185501d45468/Source/Bogus.Tests/SchemaTests/LocaleSchemaTests.cs#L77-L95
**HOWEVER,**
the `jdc` we get from `this.defaultResolver.ResolveContract(type)` happens to rely on an implementation that returns a **cached** contract if possible.
> Verify's [default contract resolver](https://github.com/VerifyTests/Verify/blob/main/src/Verify/Serialization/CustomContractResolver.cs) has logic for its own scrubbing and ignoring settings,
but it's still relying on the base implementation from Argon:
https://github.com/SimonCropp/Argon/blob/main/src/Argon/Serialization/DefaultContractResolver.cs#L55-L56\
☝🏽 And I think _this_ is the cache.
Anyway, if we're reassigning the `jdc.InterceptSerializeItem` **on the same `jdc` object**, then the **defaultIntercept()** in the callback is now what we _previously_ assigned it to.
This means that when the `InterceptSerializeItem` is run,
if it calls `defaultIntercept()`,
then _that_ `defaultIntercept()`, will call _its_ `defaultIntercept()`, which calls _its_ `defaultIntercept()`...
...until it runs the original `InterceptSerializeItem` implementation we want from Verify.
In the case of our en locale JSON, we have 1000s of items - a `defaultIntercept()` will be referencing 1000s of defaultIntercept() calls, eating up stack memory I guess.
### Known Workarounds
We only need to assign `InterceptSerializeItem` once, assuming it's not replaced with a different implementation or we get a different Contract.
We can add a class variable to save the callback, so that next time we can see if it's already been updated (by us).
If it hasn't we can do the assignment.
```C#
private InterceptSerializeDictionaryItem? updatedIntercept;
```
...
```C#
if (contract is JsonDictionaryContract jdc && jdc.InterceptSerializeItem != updatedIntercept)
{
var defaultIntercept = jdc.InterceptSerializeItem;
jdc.InterceptSerializeItem = (key, val) =>
{
...
};
this.updatedIntercept = jdc.InterceptSerializeItem;
```
### Could you help with a pull-request?
Yes
Contributor guide
Research direction
Start at Source/Bogus.Tests/SchemaTests/LocaleSchemaTests.cs, especially InterceptedContractResolver.ResolveContract() around lines 77-95, and review the linked Verify and Argon resolver implementations. Run the locale tests with dotnet test or the documented dotnet nuke commands; done means the en locale tests pass on macOS without a stack overflow.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- testing
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 35/100