Skip to content

Drop redundant fixed in HttpSys SetDataChunkWithPinnedData#66578

Open
unsafePtr wants to merge 1 commit intodotnet:mainfrom
unsafePtr:feature/httpsys-drop-redundant-fixed
Open

Drop redundant fixed in HttpSys SetDataChunkWithPinnedData#66578
unsafePtr wants to merge 1 commit intodotnet:mainfrom
unsafePtr:feature/httpsys-drop-redundant-fixed

Conversation

@unsafePtr
Copy link
Copy Markdown

Description

Drops the fixed (byte* ptr = bytes) block from the two SetDataChunkWithPinnedData helpers in HttpSys. The pin was redundant: the implementation captures ptr into chunk.pBuffer for HTTP.SYS to read later, so the fixed scope ends before the value is consumed. Safety always relied on the caller keeping the data alive, not the fixed block — the helper's name reflects that contract.

All 5 call sites verified

Call site Data Why already at a stable address
ResponseBody.cs:196, :248, :252 Helpers.ChunkTerminator / CRLF "...\r\n..."u8 — embedded in assembly RVA section, never moved
ResponseBody.cs:275 span over handle.AddrOfPinnedObject() + offset GCHandle.Alloc(..., Pinned), freed by FreeDataBuffers
ResponseStreamAsyncResult.cs:70, :134 Helpers.CRLF same UTF-8 literal

UTF-8 literals ("..."u8) live in assembly metadata, not on the GC heap. The GCHandle-pinned site keeps the buffer alive for the lifetime of the chunk's use.

Behavior preservation

fixed (byte* ptr = bytes) returns null when bytes.IsEmpty, regardless of how the span was constructed. MemoryMarshal.GetReference(bytes) does not — per its documentation, for an empty span built from a non-null pointer (e.g. a zero-length ArraySegment of a GCHandle-pinned array) it returns a non-null ref. The bytes.IsEmpty ? null : ... guard preserves the original null-on-empty semantic.

Consistency with existing code

ResponseStreamAsyncResult.cs:80 already uses the equivalent pattern for an externally-pinned array — (void*)Marshal.UnsafeAddrOfPinnedArrayElement(...) with no fixed block. The two helpers were the outliers in this file. This change aligns them with that neighboring pattern.

No behavior change at runtime; pure cleanup.

@github-actions github-actions Bot added the area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions label May 4, 2026
@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label May 4, 2026
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Thanks for your PR, @unsafePtr. Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

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 community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant