Skip to content

Conversation

@Tratcher
Copy link
Member

@Tratcher Tratcher commented May 3, 2022

Allow HttpSys zero-byte reads

Allow zero length reads from the Http.Sys request body stream

Description

AspNetCore and YARP have started to take advantage of the zero-byte-read pattern, where zero length buffers are passed to read operations to be notified when data is available. This allows the caller to avoid allocations and pinning on slow/idle streams like gRPC and WebSockets.

The Http.Sys server request body stream does not currently allow this. First, it performs argument checks that disallow a zero length buffer. Also, when the native read completes it returns ERROR_MORE_DATA rather than SUCCESS. This needs to be handled by the stream or it will throw an IOException.

Fixes #41305

Customer Impact

Since YARP 1.1 adopted the zero-byte-read pattern for performance reasons customers can no longer use it with our Http.Sys server.

Regression?

  • Yes
  • No

Risk

  • High
  • Medium
  • Low

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

@Tratcher Tratcher added the Servicing-consider Shiproom approval is required for the issue label May 3, 2022
@Tratcher Tratcher added this to the 6.0.x milestone May 3, 2022
@Tratcher Tratcher self-assigned this May 3, 2022
@ghost ghost added the area-runtime label May 3, 2022
@ghost
Copy link

ghost commented May 3, 2022

Hi @Tratcher. If this is not a tell-mode PR, please make sure to follow the instructions laid out in the servicing process document.
Otherwise, please add tell-mode label.

@ghost
Copy link

ghost commented May 3, 2022

Hi @Tratcher. Please make sure you've updated the PR description to use the Shiproom Template. Also, make sure this PR is not marked as a draft and is ready-to-merge.

To learn more about how to prepare a servicing PR click here.

@Tratcher Tratcher marked this pull request as ready for review May 3, 2022 20:00
Copy link
Member

@halter73 halter73 left a comment

Choose a reason for hiding this comment

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

Backport of #41332.

@rbhanda rbhanda modified the milestones: 6.0.x, 6.0.6 May 5, 2022
@rbhanda rbhanda added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels May 5, 2022
@dougbu dougbu merged commit 4dae9d4 into dotnet:release/6.0 May 5, 2022
@Tratcher Tratcher deleted the tratcher/release/6.0/httpsys0b branch May 5, 2022 17:51
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions Servicing-approved Shiproom has approved the issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants