OData / OData/AspNetCoreOData

Remove StreamWrapper workaround that does sync-over-async

Open
#997 0 comments 2 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

async P3
Dominant language
C#
Stars
505
Forks
186
PR merge metrics
No merged PRs in 30d

Description

We have a StreamWrapper workaround in the library that wraps around he response stream, intercepts calls to synchronous I/O and calls asynchronous I/O with Wait() or Result, e.g. Flush() calls FlushAsync().Wait(): https://github.com/OData/AspNetCoreOData/blob/c2b19f365cc650ce8140fb9f9ab9a61bb0f426d8/src/Microsoft.AspNetCore.OData/Formatter/ODataOutputFormatterHelper.cs#L244

The purpose of this wrapper is to work around the fact that ASP.NET Core by default does not allow synchronous I/O, but ODataMessageWriter and CsdlWriter have cases where they perform synchronous I/O in async code. However, this workaround can cause more harm than even having synchronous I/O since it would lead to blocking two threads where synchronous code would have blocked one. See: https://github.com/davidfowl/AspNetCoreDiagnosticScenarios/blob/master/AsyncGuidance.md#avoid-using-taskresult-and-taskwait

Since we have recently prioritized removing synchronous I/O in ODL's async APIs, we should also remove this workaround and fix any sync I/O errors that keep us from removing this workaround.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in src/Microsoft.AspNetCore.OData/Formatter/ODataOutputFormatterHelper.cs around the StreamWrapper implementation and review the ODataMessageWriter and CsdlWriter async paths. Identify the synchronous I/O errors that prevent removal, then verify that the workaround is gone and those async paths no longer require it.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
api, backend
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.