Skip to content

ReadPipesToBuffers: remove unneeded DangerousAddRef/DangerousRelease from ReadPipesToBuffers.#127553

Closed
tmds wants to merge 1 commit intodotnet:mainfrom
tmds:remove-dangerousaddref-readpipestobuffers
Closed

ReadPipesToBuffers: remove unneeded DangerousAddRef/DangerousRelease from ReadPipesToBuffers.#127553
tmds wants to merge 1 commit intodotnet:mainfrom
tmds:remove-dangerousaddref-readpipestobuffers

Conversation

@tmds
Copy link
Copy Markdown
Member

@tmds tmds commented Apr 29, 2026

@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label Apr 29, 2026
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

outputHandle.DangerousAddRef(ref outputRefAdded);
errorHandle.DangerousAddRef(ref errorRefAdded);

ReadPipes(outputHandle, errorHandle, timeoutMs,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ReadPipes on Unix does DangerousGetHandle . What is going to make that safe when this DangerousAddRef is deleted?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

You're right. I thought the add ref was to avoid the non-owning handle that was added in #126807 to lose ownership. It is also used for other things.

@tmds
Copy link
Copy Markdown
Member Author

tmds commented Apr 29, 2026

Closing. The code depends on the DangerousAddRef for polling the fds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-System.Diagnostics.Process 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.

2 participants