Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release/7.0-staging] Fix JsonDocument thread safety. #92831

Merged
merged 2 commits into from
Sep 30, 2023

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Sep 29, 2023

Backport of #76716 to release/7.0-staging

/cc @eiriktsarpalis

Customer Impact

Even though the JsonDocument/JsonElement types are documented as immutable/thread safe values, the current implementation suffers from a concurrency bug that can result in incorrect strings being returned when multiple threads attempt to call the GetString() method on the same document. This can result in nondeterministic bugs including potential privacy or security problems.

Testing

Added an OuterLoop test validating the absence of the problem.

Risk

Low. This PR removes the cache that was causing the issue. We have concluded that the removal of the cache should not contribute to any perf regressions in real-life scenaria.

Fix #92808

@ghost
Copy link

ghost commented Sep 29, 2023

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #76716 to release/7.0-staging

/cc @eiriktsarpalis

Customer Impact

Testing

Risk

IMPORTANT: If this backport is for a servicing release, please verify that:

  • The PR target branch is release/X.0-staging, not release/X.0.

  • If the change touches code that ships in a NuGet package, you have added the necessary package authoring and gotten it explicitly reviewed.

Author: github-actions[bot]
Assignees: -
Labels:

area-System.Text.Json

Milestone: -

@eiriktsarpalis eiriktsarpalis self-assigned this Sep 29, 2023
@eiriktsarpalis eiriktsarpalis added this to the 7.0.x milestone Sep 29, 2023
@eiriktsarpalis eiriktsarpalis added the Servicing-consider Issue for next servicing release review label Sep 29, 2023
@ericstj ericstj added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Sep 30, 2023
@ericstj ericstj merged commit c7425f7 into release/7.0-staging Sep 30, 2023
116 of 120 checks passed
@jkotas jkotas deleted the backport/pr-76716-to-release/7.0-staging branch October 13, 2023 01:49
@ghost ghost locked as resolved and limited conversation to collaborators Nov 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Text.Json Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Request to backport JsonDocument thread safety fix to System.Text.Json v7 (PR#76716)
2 participants