Skip to content
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

HTTP/2 ConnectionHandler close cleanup #3865

Closed
wants to merge 1 commit into from

Conversation

Scottmitch
Copy link
Member

Motiviation:
The connection handler stream close operation is unconditionally adding a listener object to a future. We may not have to add a listener at all because the future has already been completed.

Modifications:

  • If the future is done, directly invoke the logic without creating/adding a new listener.

Result:
No need to create/add listener if the future is already done in close logic.

@Scottmitch Scottmitch self-assigned this Jun 5, 2015
@Scottmitch Scottmitch added this to the 4.1.0.Beta6 milestone Jun 5, 2015
@Scottmitch
Copy link
Member Author

@nmittler PTAL.

Motiviation:
The connection handler stream close operation is unconditionally adding a listener object to a future. We may not have to add a listener at all because the future has already been completed.

Modifications:
- If the future is done, directly invoke the logic without creating/adding a new listener.

Result:
No need to create/add listener if the future is already done in close logic.
@normanmaurer
Copy link
Member

@Scottmitch looks good

@nmittler
Copy link
Member

nmittler commented Jun 8, 2015

@Scottmitch LGTM

@Scottmitch
Copy link
Member Author

Cherry-picked 4.1 (49b0d6e) master (2449760)

@Scottmitch Scottmitch closed this Jun 8, 2015
@Scottmitch Scottmitch deleted the http2_shutdown_cleanup branch June 8, 2015 14:48
Scottmitch added a commit that referenced this pull request Jun 8, 2015
Motiviation:
#3865 was merged from a machine with old code. A test case that was updates was not merged.

Modifications:
- Merge the missing test case updates

Result:
Test case no longer fails.
Scottmitch added a commit that referenced this pull request Jun 8, 2015
Motiviation:
#3865 was merged from a machine with old code. A test case that was updates was not merged.

Modifications:
- Merge the missing test case updates

Result:
Test case no longer fails.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants