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

Don't re-enter the consume callback (fixes #388). #393

Merged
merged 2 commits into from
Nov 24, 2015

Conversation

vqvu
Copy link
Collaborator

@vqvu vqvu commented Nov 18, 2015

Under certain circumstances, it was possible for the consume callback to be called while it was still executing. This can cause the source to be consumed even when there is backpressure. The result is that anything consumed while there is backpressure is thrown away.

More detailed explanation here: #388 (comment)

@@ -429,6 +429,75 @@ exports['consume - fork after consume should not throw (issue #366)'] = function
}
}

exports['race'] = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe put a reference to the PR number here?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, so you did!

@vqvu
Copy link
Collaborator Author

vqvu commented Nov 19, 2015

I added the PR number to the inner test name.

@vqvu vqvu force-pushed the consume-backpressure-fix branch from 0367027 to c6729ff Compare November 19, 2015 02:30
@vqvu vqvu force-pushed the consume-backpressure-fix branch from c6729ff to d66ae44 Compare November 19, 2015 02:33
@svozza
Copy link
Collaborator

svozza commented Nov 24, 2015

LGTM.

vqvu added a commit that referenced this pull request Nov 24, 2015
Don't re-enter the consume callback (fixes #388).
@vqvu vqvu merged commit 2d84636 into caolan:master Nov 24, 2015
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.

2 participants