azure-health-data-services-toolkit icon indicating copy to clipboard operation
azure-health-data-services-toolkit copied to clipboard

Race condition in OperationContext - SetContentAsync fails while reading large payloads

Open radupurdea opened this issue 1 year ago • 7 comments

Describe the bug When using the WebPipeline (with a simple binding, and no channels or filters), with a large request body (a bundle of 200kb, let's say), there is a race condition while reading the request body and executing the binding, causing the following exception: System.InvalidOperationException : The stream was already consumed. It cannot be read again.

To Reproduce To reproduce, POST a series of large Bundles (transaction or batch). You should use a docker container (linux) if debugging locally, as it seems it is easier to reproduce in this.

  1. Create a console app that would call your WebPipeline app with a large bundle for 100 times (in a simple for loop).
  2. Sample bundle, but add more resources so the total bytes are over 200KB: https://www.hl7.org/fhir/bundle-transaction.json.html
  3. Wait for the console app to process request.
  4. The WebPipeline will randomly throw System.InvalidOperationException : The stream was already consumed. It cannot be read again.

Expected behavior The pipeline should be able to handle all requests.

Actual behavior The pipeline would attempt to execute the binding while the request is still being read, causing the HttpClient to fail while preparing the content, as it is already read (by the OperationContext).

Environment summary SDK Version: dotnet 8.0.8 OS Version: Linux in Docker Compute Platform: (ASP.NET WebApi)

Additional context

The problem is that you are attempting to read the request body in the constructor of the OperationContext, using a GetAwaiter. This does not get correctly awaited, so the Binding is already fired, while the ReadAsByteArrayAsync is still reading.

https://github.com/microsoft/azure-health-data-services-toolkit/blob/f03e9ed13f853dbd8eb95dcf096f26442ac20638/src/Microsoft.AzureHealth.DataServices.Core/Pipelines/OperationContext.cs#L39

Sample trace (added logs on OperationContext constructor and WebBinding's ExecuteAsync):

OperationContext SetContentAsync was called.
WebBinding ExecuteBindingAsync was called.
OperationContext SetContentAsync finished.
Exception thrown: 'System.Net.Http.HttpRequestException' in System.Private.CoreLib.dll: 'An error occurred while sending the request.'

radupurdea avatar Aug 20 '24 13:08 radupurdea

Hi @radupurdea, thanks for submitting your issue! Can you let me know more information on where exactly you are getting the error in the Toolkit?

evachen96 avatar Aug 22 '24 20:08 evachen96

@radupurdea - Could you also try modifying the Quickstart sample to post the bundle, and see if that gives you an error? Here are some pointers on how to modify the Quickstart sample for this use case:

  1. File: QuickstartFilter.cs update the line 44 to : if (context.Request.Method == HttpMethod.Post)
  2. File: QuickstartFunction.cs Update lines 30 and 31 to reflect the changes necessary for bundle ingestion. [Function("/")] public async Task<HttpResponseData> PatientPost([HttpTrigger(AuthorizationLevel.Function, "post")] HttpRequestData req)

Let me know if that works!

evachen96 avatar Aug 26 '24 17:08 evachen96

@evachen96 yes, I will try and prepare a reproduceable demo.

radupurdea avatar Aug 27 '24 15:08 radupurdea

@radupurdea following up with the race-condition issue you reported. We still have not been able to reproduce your issue. Were you able to create a reproduceable demo?

erikhoward avatar Sep 04 '24 13:09 erikhoward

Closing this issue for now, feel free to reopen if you have a repro later @radupurdea!

evachen96 avatar Sep 19 '24 15:09 evachen96

Hey @evachen96 and @erikhoward I finally got some time to help in this.

You can find the code to reproduce here: https://github.com/radupurdea/azure-healthdata-toolkit-stream-issue

There are instructions in the readme: https://github.com/radupurdea/azure-healthdata-toolkit-stream-issue/blob/main/README.md

Let me know if I can help further.

Edit: Please re-open this issue as I cannot seem to do it.

radupurdea avatar Oct 04 '24 15:10 radupurdea

Thanks for the repro @radupurdea ! Our team will take a look and come back with any questions.

evachen96 avatar Oct 04 '24 17:10 evachen96

Hi @radupurdea , we have merged the fix for this in https://github.com/microsoft/azure-health-data-services-toolkit/pull/143. Thanks for submitting the issue and collaborating on the PR!

As a note, we currently have another known issue where Github changes are not being reflected in our package (https://github.com/microsoft/azure-health-data-services-toolkit/issues/135). To get the latest changes, please pull the Github source code directly. Thanks!

evachen96 avatar Oct 15 '24 18:10 evachen96