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

Fixes wrong request accounting in AbstractOnSubscribe #2854

Merged
merged 1 commit into from
Apr 7, 2015

Conversation

akarnokd
Copy link
Member

@akarnokd akarnokd commented Apr 3, 2015

Fixes #2853.

@akarnokd akarnokd added the Bug label Apr 3, 2015
break;
}
}
} else
if (n > 0 && state.requestCount.getAndAdd(n) == 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use BackpressureUtils.getAndAdd here to avoid request overflow?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks. I thought I added it because of the visibility changes, but it appears it got lost somewhere.

@akarnokd akarnokd force-pushed the AbstractOnSubscribeRequestFix branch from c7e3b2f to 622b0a9 Compare April 3, 2015 09:37
if (!state.subscriber.isUnsubscribed()) {
do {
if (n > 0 && BackpressureUtils.getAndAddRequest(state.requestCount, n) == 0) {
// fast-path
Copy link
Collaborator

Choose a reason for hiding this comment

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

move this comment down a line (inside the next if block) for clarity?

@davidmoten
Copy link
Collaborator

LGTM beyond trivial changes mentioned above. Thanks @akarnokd for fixing quickly (as usual)!

@akarnokd akarnokd force-pushed the AbstractOnSubscribeRequestFix branch from 622b0a9 to 674327d Compare April 7, 2015 09:42
@akarnokd
Copy link
Member Author

akarnokd commented Apr 7, 2015

Done.

@akarnokd akarnokd force-pushed the AbstractOnSubscribeRequestFix branch from 674327d to 31339a2 Compare April 7, 2015 09:44
akarnokd added a commit that referenced this pull request Apr 7, 2015
Fixes wrong request accounting in AbstractOnSubscribe
@akarnokd akarnokd merged commit 0507fd1 into ReactiveX:1.x Apr 7, 2015
@akarnokd akarnokd deleted the AbstractOnSubscribeRequestFix branch April 7, 2015 17:14
@benjchristensen benjchristensen mentioned this pull request Apr 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants