Skip to content

[release/5.0-preview3] Cache parameterized ctor delegates in class info rather than converter - #34353

Merged
Anipik merged 1 commit into
dotnet:release/5.0-preview3from
layomia:ctors
Apr 3, 2020
Merged

[release/5.0-preview3] Cache parameterized ctor delegates in class info rather than converter#34353
Anipik merged 1 commit into
dotnet:release/5.0-preview3from
layomia:ctors

Conversation

@layomia

@layomialayomia commented Apr 1, 2020

Copy link
Copy Markdown
Contributor

Description

Ports #34248 to preview 3.
Fixes#33928 in preview 3.

A JsonSerializer feature that enables the deserialization of POCOs using parameterized ctors was recently checked in - #33444.

CI caught an issue where JsonSerializer leaks a null ref in some cases where deserialization of objects using parameterized ctors is occurring on multiple threads: #33928. This PR fixes the issue.

Customer Impact

Prevents a null ref in multi-threaded deserialization scenarios. Users are likely to hit this when deserializing objects using parameterized ctors on mutliple threads. This proactively fixes the issue.

Regression?

This feature is shipping for the first time in this preview.

Risk

Tests have been added for this scenario.

dotnet#34248)
* Cache parameterized ctor delegates in class info rather than converter
* Address review feedback - move delegate assignment to start of deserialization
* Address review feedback - nullability
@layomialayomia added bug Servicing-consider Issue for next servicing release review area-System.Text.Json labels Apr 1, 2020
@layomialayomia added this to the 5.0 milestone Apr 1, 2020
@layomia
layomia requested a review from ahsonkhan as a code ownerApril 1, 2020 02:07
@layomialayomia self-assigned this Apr 1, 2020
@ericstjericstj added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Apr 3, 2020
@Anipik
Anipik merged commit 8f6b899 into dotnet:release/5.0-preview3Apr 3, 2020
@layomia
layomia deleted the ctors branch April 5, 2020 05:39
@ghostghost locked as resolved and limited conversation to collaborators Dec 9, 2020
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Text.JsonbugServicing-approvedApproved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@layomia@ericstj@Anipik