Skip to content

Fix for a rare race condition in System.IO.Packaging #63013

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

Merged
merged 1 commit into from
Dec 20, 2021
Merged

Fix for a rare race condition in System.IO.Packaging #63013

merged 1 commit into from
Dec 20, 2021

Conversation

themcoo
Copy link
Contributor

@themcoo themcoo commented Dec 20, 2021

Fix for a rare race condition when initializing System.IO.Packaging.PackageRelationship.s_containerRelationshipPartName._partUriExtension

In rare circumstances, when there are at least two ooxml packages created in parallel for the first time in application's lifetime, it is possible that PackageRelationship.s_containerRelationshipPartName._partUriExtension will be initialized to "els" instead of "rels". Once it's initialized like that, every Package is saved with an invalid extension for package relationships content type in [Content_Types].xml:
<Default ContentType="application/vnd.openxmlformats-package.relationships+xml" Extension="els"/>
When reading such damaged package later, the relationships are not to be found.

As this is extremely non-deterministic, no unit tests were provided. I developed a tiny app to reproduce this issue, launching it in a loop I was able to get the issue 3 times over 8000 runs.

…Packaging.PackageRelationship.s_containerRelationshipPartName._partUriExtension
@ghost ghost added area-System.IO community-contribution Indicates that the PR has been added by a community member labels Dec 20, 2021
@ghost
Copy link

ghost commented Dec 20, 2021

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

Issue Details

Fix for a rare race condition when initializing System.IO.Packaging.PackageRelationship.s_containerRelationshipPartName._partUriExtension

In rare circumstances, when there are at least two ooxml packages created in parallel for the first time in application's lifetime, it is possible that PackageRelationship.s_containerRelationshipPartName._partUriExtension will be initialized to "els" instead of "rels". Once it's initialized like that, every Package is saved with an invalid extension for package relationships content type in [Content_Types].xml:
<Default ContentType="application/vnd.openxmlformats-package.relationships+xml" Extension="els"/>
When reading such damaged package later, the relationships are not to be found.

Author: themcoo
Assignees: -
Labels:

area-System.IO, community-contribution

Milestone: -

Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you for your contribution @themcoo ! 👍

Is there any chance you could share the repro app? It would help me to estimate whether we should backport this fix.

@adamsitnik adamsitnik added this to the 7.0.0 milestone Dec 20, 2021
@adamsitnik adamsitnik merged commit 8619a63 into dotnet:main Dec 20, 2021
@themcoo
Copy link
Contributor Author

themcoo commented Dec 20, 2021

LGTM, thank you for your contribution @themcoo ! 👍

Is there any chance you could share the repro app? It would help me to estimate whether we should backport this fix.

Cool, happy to see it merged so quickly ;)

@adamsitnik I pushed my little messy proofing app to repo https://github.com/themcoo/SystemIOPackaging.Race

Just build, run run.bat and wait for the bug to occur (with message "Race happened!").
My latest 5 launches of run.bat ended with the bug occurring at the following attempts:
2302
97
334
4491
489

Maybe it's worth to mention why I was investigating this. This bug actually occurred on production release of our applications (1000+ ish instances, probably restarting daily). First time circa 10 months ago, second time a week ago (app was released a year ago). The second time made me dig into the dotnet source to find the reason behind the malformed content type xml....

@themcoo themcoo deleted the mcoo/system.io.packaging-thread-race-condition-fix branch December 29, 2021 10:12
@ghost ghost locked as resolved and limited conversation to collaborators Jan 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.IO community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants