Repository navigation
Conversation
Refactor context handling in CreateAndConfigure method to use ThreadStatic field for thread safety.
|
@dotnet-policy-service agree |
|
@commonsensesoftware just following up on this PR. Is there anything you'd like me to change or clarify before this can be merged? This is currently breaking API documentation generation unless significant workarounds are done, so getting a fix in would be really helpful. |
|
@OptoCloud thanks for taking the time put this together. It looks like a simple, straight forward approach. I intend to review and think about this a bit more to make sure this is the right direction to go. This might be a good stop gap or might be the real final answer. The internals are super wonky, but I wasn't able to get MS to unseal the options. That would make so much of this a lot less clunky. In retrospect, it works, but I'm not really pleased with the design. I've been backed up a bit lately, but I'll put eyes on this ASAP. |
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:
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