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

request(0) is supposed to work? #1956

Closed
zsxwing opened this issue Dec 12, 2014 · 6 comments
Closed

request(0) is supposed to work? #1956

zsxwing opened this issue Dec 12, 2014 · 6 comments

Comments

@zsxwing
Copy link
Member

zsxwing commented Dec 12, 2014

The docs about request doesn't say anything requirement about n

public interface Producer {

    /**
     * Request a certain maximum number of items from this Producer. This is a way of requesting backpressure.
     * To disable backpressure, pass {@code Long.MAX_VALUE} to this method.
     *
     * @param n the maximum number of items you want this Producer to produce, or {@code Long.MAX_VALUE} if you
     *          want the Producer to produce items at its own pace
     */
    public void request(long n);

}
    /**
     * Request a certain maximum number of emitted items from the Observable this Subscriber is subscribed to.
     * This is a way of requesting backpressure. To disable backpressure, pass {@code Long.MAX_VALUE} to this
     * method.
     *
     * @param n the maximum number of items you want the Observable to emit to the Subscriber at this time, or
     *           {@code Long.MAX_VALUE} if you want the Observable to emit items at its own pace
     */
    protected final void request(long n) {

I assume n must be greater than 0. But I also notice I may write something like this occasionally:

long getTheNextRequestNumber() {
  if (...) {
       ...
       return x; // x>0
  } else {
        ...
       return 0;
   }
}

long n = getTheNextRequestNumber();
request(n);

So request(0) is fine? If not, I think it's better to document it.

@benjchristensen
Copy link
Member

request(0) should do nothing, be a no-op.

@akarnokd
Copy link
Member

I concur yet some places, a request(0) may trigger a loop and decrement the request counter below zero on the exit edge or do some other sideeffects. (Search for .getAndAdd( in the code)

  • OnSubscribeCombineLatest: will subscribe to sources and fill the internal buffers but not emit anything
  • OnSubscribeRedo: will trigger a reschedule and clears the boundary indicator
  • OperatorConcat: will deliver oncompleted
  • OperatorGroupBy: does an atomic increment and atomic decrement
  • OperatorObserveOn: does schedule a thread and may decrement request below 0 for a short amount of time
  • OnBackpressureBlock: is incorrect anyway, but triggers an empty emission loop
  • OnBackpressureBuffer: triggers a drain loop and may decrement request below 0 for a short time

@benjchristensen
Copy link
Member

Then it looks like we should better document that request(0) should do nothing, and fix these places in code so they really do nothing.

Do we want to ignore a negative number as well (like 0), or throw an IllegalArgumentException? As a data point the Reactive Streams spec throws an IAE.

@zsxwing
Copy link
Member Author

zsxwing commented Dec 12, 2014

Do we want to ignore a negative number as well (like 0), or throw an IllegalArgumentException? As a data point the Reactive Streams spec throws an IAE.

Usually a negative number is not intentional, usually means a bug in some place. So I like IAE rather than ignoring them.

@benjchristensen
Copy link
Member

I'm okay with IAE.

@akarnokd
Copy link
Member

I'm using request(0) some places/tests to suppress the default behavior of requesting unlimited amount.
Negative, I think, is rather a bug than a convenience for no-op.

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

No branches or pull requests

3 participants