Skip to content

Fix race creating options for documents requested concurrently - #1229

Open
OptoCloud wants to merge 4 commits into
dotnet:mainfrom
OptoCloud:main
Open

OptoCloud wants to merge 4 commits into
dotnet:mainfrom
OptoCloud:main

Conversation

@OptoCloud

Copy link
Copy Markdown
  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

Description

VersionedOpenApiOptionsFactory is registered as a singleton but kept the context of the options being created in an instance field.
CreateAndConfigure sets the field, calls Create, and clears it again.
When the options of two documents are created at the same time, one thread can clear the field between another thread's null check and its use.

This produces one of two results, and OptionsCache stores either for the lifetime of the process:

  • a NullReferenceException in VersionedOpenApiOptionsFactory.Create, so the document endpoint returns a 500
  • Create sees a null or mismatched context and returns DefaultOptions(), so the document is served without any of the versioned configuration

The context is now stored in a [ThreadStatic] field. Create is only ever called synchronously from CreateAndConfigure on the same thread, so each thread sees only its own context.
The reset also moved into a finally block so an exception during configuration no longer leaves a stale context behind.

A regression test starts a new host 50 times, requests /openapi/v1.json and /openapi/v2.json at the same moment, and asserts that each document has its own API version and paths.
It fails against the current code with the NullReferenceException and passes with this change.

Fixes #1228

@OptoCloud

Copy link
Copy Markdown
Author

@dotnet-policy-service agree

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

VersionedOpenApiOptionsFactory race: concurrent document requests cause NullReferenceException or unconfigured document

1 participant