Skip to content

Release and dispose the native handles when releasing RequestContext #134

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 14, 2025

Conversation

mayuki
Copy link
Member

@mayuki mayuki commented May 12, 2025

No description provided.

@mayuki mayuki requested a review from Copilot May 12, 2025 10:02
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves the resource cleanup of the request context by properly releasing and disposing the native handle while also updating several tests to validate connection handling and ensure correct disposal of resources.

  • Updated test attributes to limit cross-platform execution (Windows only)
  • Added middleware to capture connection IDs and to track active connections
  • Refactored disposal logic in production code (ResponseContext and RequestContext) to ensure native handles are released appropriately

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/YetAnotherHttpHandler.Test/YetAnotherHttpHandlerTest.cs Adjusted test attributes for platform-specific execution
test/YetAnotherHttpHandler.Test/TestServerForHttp1AndHttp2.cs Added middleware to inject a connection ID header
test/YetAnotherHttpHandler.Test/Http2TestBase.cs Modified disposal of HTTP handler and client, added assertions for connection cleanup
test/YetAnotherHttpHandler.Test/Helpers/TestWebAppServer.cs Introduced active connections tracking via middleware
src/YetAnotherHttpHandler/ResponseContext.cs Ensured _tokenRegistration is disposed in all completion methods
src/YetAnotherHttpHandler/RequestContext.cs Refactored native handle disposal in Release() and removed redundant code from Dispose()
Comments suppressed due to low confidence (1)

test/YetAnotherHttpHandler.Test/YetAnotherHttpHandlerTest.cs:113

  • [nitpick] Consider adding a brief comment explaining that the test now explicitly runs only on Windows to clarify the rationale behind the platform-specific condition.
[Fact] and [OSSkipCondition(OperatingSystems.MacOSX | OperatingSystems.Linux)]

@mayuki mayuki changed the title Release and dispose the native handle of the request context when releasing RequestContext Release and dispose the native handles when releasing RequestContext May 14, 2025
@mayuki mayuki merged commit 3ab3d4d into main May 14, 2025
15 checks passed
@mayuki mayuki deleted the feature/ReleaseHandleOnCompleted branch May 14, 2025 07:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant