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.

Sign up for GitHub

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

Jump to bottom

HTTP/2 DefaultHttp2RemoteFlowController Stream writability notification broken #4587

Closed
Scottmitch opened this issue Dec 17, 2015 · 3 comments
Closed

HTTP/2 DefaultHttp2RemoteFlowController Stream writability notification broken #4587

Scottmitch opened this issue Dec 17, 2015 · 3 comments
Assignees
@Scottmitch
Labels
defect
Milestone
4.1.0.CR1

Comments

@Scottmitch
Copy link
Member

DefaultHttp2RemoteFlowController.ListenerWritabilityMonitor no longer reliably detects when a stream's writability change occurs. ListenerWritabilityMonitor was implemented to avoid duplicating iteration over all streams when possible and instead was relying on the PriorityStreamByteDistributor to call write for each stream during its iteration process. However the new StreamByteDistributor classes do not do an iteration over all active streams and so this assumption is now invalid.

The impact is isolated to stream writability change notifications and has no impact unless you explicitly add a listener, and use an allocator other than PriorityStreamByteDistributor.
@Scottmitch Scottmitch self-assigned this Dec 17, 2015
This was referenced Dec 17, 2015
Closed
Closed
@Scottmitch Scottmitch added this to the 4.1.0.CR1 milestone Dec 17, 2015
@Scottmitch
Copy link
Member Author

@nmittler - FYI

@nmittler
Copy link
Member

@Scottmitch good find! Any thoughts on a fix?

@Scottmitch Scottmitch mentioned this issue Dec 18, 2015
Closed
@Scottmitch
Copy link
Member Author

@nmittler - Yes. See #4599

Scottmitch added a commit to Scottmitch/netty that referenced this issue Dec 18, 2015
@Scottmitch
HTTP/2 DefaultHttp2RemoteFlowController Stream writability notificati…
3e02cb6 
…on broken

Motivation:
DefaultHttp2RemoteFlowController.ListenerWritabilityMonitor no longer reliably detects when a stream's writability change occurs.

Modifications:
- Ensure writiability is reliabily reported by DefaultHttp2RemoteFlowController.ListenerWritabilityMonitor
- Fix infinite loop issue (netty#4588) detected when consolidating unit tests

Result:
Reliable stream writability change notification, and 1 less infinite loop in UniformStreamByteDistributor.
Fixes netty#4587
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Scottmitch Scottmitch

Labels
defect
Projects
None yet
Milestone
4.1.0.CR1
Development

No branches or pull requests
2 participants
@nmittler @Scottmitch