Race condition - intermittent exceptions "Operations that change non-concurrent collections must have exclusive access"

[udlose]

Hi,

I was experiencing the same race condition as explained in #67 and have a small project that has been able to repro the issue (of course intermittently since it's a race condition). I have posted the sample solution here: https://github.com/udlose/AWSXRayRaceCondition

• .NET Core 7.0
• AWSXRayRecorder 2.13.0

AWSXRayRaceConditionTests.TelemetryRecorderTests.Trace CallsActionAndRecords WhenTracingIsEnabled
   Source: TelemetryRecorderTests.cs line 82
   Duration: 1 ms

  Message: 
System.InvalidOperationException : Operations that change non-concurrent collections must have exclusive access. A concurrent update was performed on this collection and corrupted its state. The collection's state is no longer correct.

  Stack Trace: 
Dictionary`2.TryInsert(TKey key, TValue value, InsertionBehavior behavior)
Dictionary`2.set_Item(TKey key, TValue value)
JsonMapper.RegisterExporter[T](ExporterFunc`1 exporter) line 922
JsonSegmentMarshaller.ctor() line 48
UdpSegmentEmitter.ctor() line 45
AWSXRayRecorder.ctor() line 47
TelemetryRecorderTests.ctor() line 25
RuntimeType.CreateInstanceDefaultCtor(Boolean publicOnly, Boolean wrapExceptions)

[udlose]

It appears the root cause of the race is in LitJson which appears to be used by AWSXRayRecorder. I've opened a PR with LitJson project. You would obviously need to consume the latest version once they accept the PR and release the fix.

• LitJson Issue: Race conditions affecting JsonMapper and its usage of static members w/o synchronization LitJSON/litjson#143
• LitJson PR: Add thread safety to fix race conditions around usage of static IDict… LitJSON/litjson#142

[jj22ee]

Hi @udlose,

It looks like your PR to LitJson hasn't gotten any traction. We've faced a similar issue with this dependency before and that is why we have the source code of LitJson directly in this repository (aws-xray-sdk-dotnet/sdk/src/Core/ThirdParty/LitJson/JsonMapper.cs).

Can you make your PR changes to LitJson to aws-xray-sdk-dotnet/sdk/src/Core/ThirdParty/LitJson/ as well, so you do not need to further wait for the maintainers of LitJson to approve your PR? Also can you apply the changes without the formatting changes to make the review easier?

[Compufreak345]

Hi @udlose ,
we are facing the same issue here, so I would like to second the ask from @jj22ee .

You would help us a ton if you could do that :)

Thanks & best regards,
Christoph

[udlose]

I've updated my PR to LitJson (LitJSON/litjson#142) and will re-create a duplicate PR against this repo tonight.

[udlose]

@Compufreak345 , @jj22ee PR submitted. I noticed there was some code in the latest JsonMapper.cs that wasn't included here so I left that in the PR. Feel free to remove if desired.

[jj22ee]

Thanks for submitting the PR. I see that most of the PR Checks are failing due to a ThirdParty.LitJson.JsonException. The Unit Tests are failing due to a recursive (infinite?) loop in the the WriteValue() method. Added a comment in the PR.

[udlose]

Hi @udlose,

It looks like your PR to LitJson hasn't gotten any traction. We've faced a similar issue with this dependency before and that is why we have the source code of LitJson directly in this repository (aws-xray-sdk-dotnet/sdk/src/Core/ThirdParty/LitJson/JsonMapper.cs).

Can you make your PR changes to LitJson to aws-xray-sdk-dotnet/sdk/src/Core/ThirdParty/LitJson/ as well, so you do not need to further wait for the maintainers of LitJson to approve your PR? Also can you apply the changes without the formatting changes to make the review easier?

@Compufreak345 , @jj22ee
Just to provide an update here, my pr for the fix in LitJSON (LitJSON/litjson#142) was merged on 11/19/23 - LitJSON/litjson#142 (comment)

If you update the AWS Xray SDK to use the latest version of LitJSON v0.19, it should fix this issue.

[jj22ee]

Thanks @udlose. We cannot directly depend on LitJson as there is some custom code in our own copy of the LitJSON, so we will need to cherry-pick your specific change into our repository. I've updated your PR to do so, and restored some accidentally deleted code.

[jj22ee]

PR is merged. Once we release the next version, I can close this issue.

[jj22ee]

Closing issue since PR is merged, the next release of XRay Dotnet SDK will include this bugfix.