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

Correctly resubscribe TCPConnection to ASIO events after throttling #1558

Merged
merged 1 commit into from
Feb 6, 2017

Conversation

SeanTAllen
Copy link
Member

As part of the TCPConnection/EPoll One Shot changes that I previously
submitted, there was a bug where when the TCPConnection experiences
throttling, it won't ever get notified when we are ready to send again.

This patch addresses that in the short term. There's a PR coming from
Dipin Hora that addresses this in a better fashion and handles a
subtle race condition in One Shot ASIO events. This small patch
should suffice in the meantime and get master back to a correct state.

Close #1539

As part of the TCPConnection/EPoll One Shot changes that I previously
submitted, there was a bug where when the TCPConnection experiences
throttling, it won't ever get notified when we are ready to send again.

This patch addresses that in the short term. There's a PR coming from
Dipin Hora that addresses this in a better fashion and handles a
subtle race condition in One Shot ASIO events. This small patch
should suffice in the meantime and get master back to a correct state.
@SeanTAllen SeanTAllen added the changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge label Feb 5, 2017
@jemc
Copy link
Member

jemc commented Feb 6, 2017

Would be nice if we had some tests that covered this, but I believe it was discussed in #1539 that it wouldn't be practical.

@jemc jemc merged commit 93fb863 into master Feb 6, 2017
@jemc jemc deleted the issue-1539 branch February 6, 2017 17:56
ponylang-main added a commit that referenced this pull request Feb 6, 2017
@SeanTAllen
Copy link
Member Author

@jemc i think what we need are Spy type tests that verify the contract is working correctly. We are lacking a bit on tooling for that at the moment. I started a discussion with Sylvan over the weekend on what would be required to write a nice spy tool for Pony.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants