Documentation of intent for each and every rule #339

viktorklang · 2017-01-27T00:17:22Z

Dear @reactive-streams/contributors,

we've been talking for ages about adding an intents-section for every spec rule,
and during a cross-atlantic flight I finally found the time to write this up.

This PR is not intended for merging, but is rather a conversation to figure out the proper wording/intent for each of the spec rules.

The idea would be to iterate on that, and then I'll create a real PR against the README.md

Additionally, I've found some opportunities to clarify some of the spec rules, and I think that'd be of general interest.

NONE OF THE THINGS PROPOSED HEREIN CHANGES ANY SEMANTICS OF THE SPEC WHATSOEVER.

√

viktorklang · 2017-01-27T00:53:44Z

RFC.md

+    - `terminal state` / `end-of-stream` <-- change spec to use one of these
+    - `responsivity` / `non-obstructing` <-- change spec to use one of these
+    - `call`/`invoke` <-- change spec to use one of these
+


Possible Issues that this proposal would resolve:

#334
#328
#325
#324
#323
#322
#321
#319
#290

viktorklang · 2017-02-10T09:23:06Z

@reactive-streams/contributors ping

viktorklang · 2017-02-10T09:24:47Z

@reactive-streams/contributors Esp. in addition to the intents, have a look at the FIXMEs

DougLea · 2017-02-10T12:33:15Z

Thanks for doing this. Modulo a copy-edit pass or two, I like it. But I'm not sure about the best way to present it. Intermixing it with spec is probably best for now, but might be reconsidered if we ever decide to put out a definitive guide, that might include coding examples etc.

smaldini · 2017-02-10T12:40:20Z

👍 Thanks @viktorklang as usual ! nice wording polish

viktorklang · 2017-02-10T13:18:30Z

@DougLea @smaldini Thank you! I was considering whether it would be possible to add the intents either as a part of the "spec table" in the README.md or as tooltips on each of the spec rules. Perhaps a bit too hidden…

@reactive-streams/contributors
If we're fine with the basic explanations (wordsmithing will take place I'm sure of it) I'll open a more serious PR against the README.md and break out the FIXMEs in the doc pertaining to the rules themselves as Issues. Sounds good?

akarnokd · 2017-02-10T13:22:14Z

RxJava 2 doesn't precisely follow some of the rules for performance reasons so we have to provide the operator strict() to make sure the TCK tests pass. I'd like to have some relaxations to these rules:

§1.3: `onNext` should not be called concurrently until `onSubscribe` returns

Rationale: the downstream may issue a request() which the upstream may service from another thread before the middle-level onSubscribe returned.

RxJava 2: not a problem, our operators behave in a thread safe manner even after onSubscribe on the downstream has been called.

TCK: RxJava 2's observeOn and delay operators fail probabilistically.

Workaround: deferred requesting: collect up any request calls from downstream and re-emit them after the downstream returned from onSubscribe.

Relax: it is allowed to call onNext synchronously from onSubscribe, allow calling onNext from onSubscribe asynchronously before onSubscribe returns to its caller.

§2.3: onError or onComplete must not call cancel/request

Rationale: avoid post-terminal cycles due to cancel/request

Problem: the rule is in conflict with §1.6/§2.4 (terminal events imply cancelled Subscription) + §3.6 / §3.7 (cancelled Subscription should ignore further request/cancel). In addition, cancellation may happen from an asynchronous source while onError or onComplete is being executed.

RxJava 2: receiving terminal events imply the Subscription is in cancelled state and any further call to request/cancel on it is no op. Some operators use it for mass cancellation whether or not a particular source is known to be terminated.

TCK: can't remember any specific failures due to this

Relax: when onError and onComplete is called, the associated Subscription should be considered cancelled and calls to request/cancel to be interpreted as no-ops.

§3.9: negative requests should emit an onError(IllegalArgumentException)

Rationale: protocol violations should notify and cancel sequence and notify downstream about the error

Problem: bad requests may happen concurrently with emissions to onXXX methods and to honor §1.3 (no concurrent onXXX invocations), one has to serialize all emissions to downstream which is quite expensive to ensure. Non-positive requests are bugs in operators and should be fixed on the library level instead of anticipated.

RxJava 2: we consider non-positive requests bugs that get signalled to a global error handler but the stream remains intact by default.

TCK: we have to use strict mode to pass the TCK as it explicitly does bad requests and expects onError to come back.

Relax: Don't mandate an onError when a non-positive request is found but use the "the caller MUST raise this error condition in a fashion that is adequate for the runtime environment" backdoor.

§2.12: call `onSubscribe` once per `Subscriber` instance

Rationale: discourage reusing the same, stateful Subscriber on multiple Publishers. Such reuse would require adequate merging of onXXX calls from the multiple sources and ensure sequentiality

RxJava 2: we don't check and sometimes we do reuse the same Subscriber for multiple Publishers to save on unnecessary allocation and the Subscriber implementation ensures proper serialization towards its downstream.

TCK: Not sure if this is tested or not, strict creates a new Subscriber for each incoming Subscriber thus for the upstream, it hides the fact that the end Subscriber instance is always the same after all.

Relax: Not sure how to word this without "object equality" but it is the Publisher.subscribe() method's responsibility that when the subscription chain is realized it doesn't call Subscriber.onSubscribe more than once, for example:

// this is not okay

Publisher<Integer> p = s -> {
   s.onSubscribe(new SomeSubscription());
   s.onSubscribe(new SomeOtherSubscription());
};

// this isn't okay either

Publisher<Integer> q = s -> {
   s.onSubscribe(new SomePublisher());

   otherSource.subscribe(s); // that other source will call onSubscribe yet again
};

// this may be legal but discouraged:

Publisher<Integer> z = s -> s.onSubscribe(new YetAnotherSubscription());

Subscriber<Integer> sub = new Subscriber<Integer>() {
   CompositeSubscription cs = new CompositeSubscription();

   @Override
   public void onSubscribe(Subscription s) {
      cs.add(s);
      s.request(Long.MAX_VALUE);
   }

   // etc.
};

z.subscribe(sub);
z.subscribe(sub);
z.subscribe(sub);

sub.cs.cancel();

akarnokd · 2017-01-27T09:24:20Z

RFC.md

+`A Publisher MAY signal less (FIXME: FEWER) onNext than requested and terminate the Subscription by calling onComplete or onError.`
+
+Comments:
+Publisher cannot guarantee that it will be able to produce the number of elements requested; it simply might not be able to produce them all; it may be in a failed state; it may be empty or otherwise already completed.


Publisher may not be able to emit at most the requested amount and may chose to signal onError. Example: high frequency periodic timer.

akarnokd · 2017-01-27T09:52:11Z

RFC.md

+`While the Subscription is not cancelled, calling Subscription.cancel MAY cause the Publisher, if stateful, to transition into the shut-down state if no other Subscription exists at this point [see 1.9].`
+
+Comments:
+The intent of this rule is to allow for Publishers to not accept new Subscribers in response to a cancellation signal. [FIXME: is this overly restrictive? Shouldn't a Publisher be allowed to deny new Subscribers at its own discretion?]


It's odd. I haven't met a Publisher that disallowed further Subscribers because one of the older Subscribers cancelled its own Subscription. Even if, newer Subscribers must be rejected with an onError so they don't hang on data that never comes.

smaldini · 2017-02-10T13:59:11Z

@akarnokd effectively this is affecting both reactive-streams-common based projects so it includes reactor 3 and I do concur with you on these slight amendments with maybe room to discuss more 1.3 and 2.12.

DougLea · 2017-02-10T14:12:08Z

@akarnokd Several of these items are currently phrased in terms of actions that shouldn't occur, but (as once discussed) it may be more useful to spec them in terms of what may vs may not happen when these cases/actions occur. Maybe you could try some candidate rephrasings along these lines.

viktorklang · 2017-02-13T12:12:06Z

@akarnokd

My general stance at this point in time: backwards compatibility SHOULD be maintained.

Thanks for providing input (would love some input on my descriptions tho), I hope the following addresses your comments:

Re: 1.3: Relaxing the rule as you propose would not only be backwards incompatible, it would also mean that every Subscriber (even synchronous ones) need to employ atomics or synchronization to prevent corruption in the face of concurrent signalling. This of course assuming I don't misinterpret your request.

Re 2.3: There is no conflict here. These rules specify that publishers and Subscribers must mutually be "hands-off" the Subscription in the case of a terminal signal being received.

Re: 3.9: In this case I'd be fine with RECOMMEND IllegalstateException and offer the escape hatch of logging or otherwise. This would also be backwards compatible.

Re: 2.12: This rule applies for generic Subscribers, if your Publisher knows that certain types of Subscribers are safe to reuse or are otherwise reentrant, you may of course apply any logic you want. RS only concerns itself by specifying what a generic Publisher can expect from a generic Subscriber and vice versa.

@smaldini @ktoso @rkuhn @DougLea Thoughts?

akarnokd · 2017-02-13T12:42:22Z

@DougLea I'm still not a native English speaker, all I can offer are some points to consider when one better with words does rephrase. Besides, I like rewriting things from scratch which is not desired here.

@viktorklang

§1.3: RS is aimed at library writers, RxJava 2 and Reactor 3 are already relaxed this way and Akka-Stream's converters don't seem to be affected. Often there is no need for extra atomics in end-Subscribers but making sure necessary initialization happens in onSubscribe before issuing the first s.request(n) call.

§2.3: The rule says "must not" and either

dropping it as §1.6, §2.4, §3.6 and §3.7 are enough or
reinforcing them with a different phrasing: "When onError and onComplete is called, the associated Subscription should be considered cancelled and calls to request/cancel to be interpreted as no-ops".

§3.9: The emphasis is on not mandating a call to onError. The drawback is that the TCK has to be changed as well by either dropping the check or offering an out-of-band method to receive the exception.

§2.12: this affects cross-library behavior, i.e., what happens if I concatenate 2 Akka-Streams sources? Will it complain that the RxJava 2's ConcatArraySubscriber instance has already been seen?

viktorklang · 2017-02-13T12:56:35Z

@akarnokd

Re: 1.3: To clarify, you agree that the change as not backwards compatible and that it will require atomics/synchronization in all Subscriber implementations which are stateful?

Re: 2.3: So we agree that there is no conflict? (I.e. only an opportunity to either simplify or reword, maintaining the exact same semantics.)

Re: 3.9: I think it is much better to RECOMMEND a specific behavior over a non-specific one, don't you agree?

Re: 2.12: Good point—yes, you would have to clone it to give it to a generic Publisher implementation more than once (i.e. no matter if Akka Streams is used or not). If you're proposing removing the check for duplicate use, how would you detect/report multi-subscription?

akarnokd · 2017-02-13T13:05:53Z

§1.3: In your sense, it is not backwards compatible and it may need extra synchronization or atomics in existing Subscribers. This could be deferred till 2.0.

§2.3: I think it is a conflict in the current wording of the rules which should be resolved via rewording §2.3.

§3.9: RECOMMEND yes and adjust the TCK to have this particular test as optional (I don't know how to do that).

§2.12: It is the responsibility of the Subscriber to detect unsupported multiple calls to its onSubscribe method. Publisher should not do identity checks.

smaldini · 2017-02-13T13:12:06Z

@viktorklang @akarnokd

Re: 1.3: For reentrant publisher we at least guarantee one level of reentrance, usually if you do parent.request(n) as the last method call in onSubscribe we already established before that it is side effect free but not a guarantee per se. I'm neutral on this behavior wording because effectively besides the TCK being strict about it, that's what happens even in synchronous case if you do request last. Keeping the current wording, we can precise "the implementor COULD achieve a similar side-effect free onSubscribe intended by this rule by making sure any Subscription#request interaction IS the last statement of an onSubscribe callback". Still that requires a relaxed TCK on this which actively use barriers to verify it.

Re: 2.3: It's a wording problem it seems, excluding synchronous Publisher-Subscriber behavior which could really provide the guarantee. But the spec aims higher and we could just use "SHOULD NOT" and explain why this is hard to actually enforce due to possible asynchronous event flow since only onXxxx are enforced with the non concurrency rule.

Re: 3.9: Recommending to trace this behavior for debug purposes but not fail unless explicitly specified by the implementor API ?

Re: 2.12: We don't need to enforce a check on a publisher for this, validating double onSubscribe from a Subscriber perspective is easy a check to delegate to a Subscriber.

viktorklang · 2017-02-13T21:18:09Z

@reactive-streams/contributors

Dear contributors,

please have a look at this updated PR, I think this is looking pretty decent: https://github.com/reactive-streams/reactive-streams-jvm/blob/69fe7a6e46911d6eb5d9979885509598717c0d77/README.md

akarnokd · 2017-02-13T21:44:22Z

README.md

 | <a name="2.12">12</a>     | `Subscriber.onSubscribe` MUST be called at most once for a given `Subscriber` (based on object equality). |
+| [:bulb:](#2.12 "2.12 explained") | *The intent of this rule is to establish that it is not to be considered intended behavior that the same Subscriber is concurrently subscribed to several Publishers.* |


The intent of this rule is to establish that it is not to be considered intended behavior that the same Subscriber is subscribed to by more than one Publisher (one after the other from the same thread or concurrently).

akarnokd · 2017-02-13T21:55:28Z

README.md

 | <a name="3.10">10</a>     | While the `Subscription` is not cancelled, `Subscription.request(long n)` MAY synchronously call `onNext` on this (or other) subscriber(s). |
+| [:bulb:](#3.10 "3.10 explained") | *The intent of this rule is to establish that it is allowed to create synchronou Publishers, i.e. Publishers who execute their logic on the calling thread.* |


typo: `synchronou" -> "synchronous".

Do I understand correctly that this rule refers to a fanout situation when calling Subscription.request() by one Subscriber it may unblock other subscribers and now the same thread multicasts to multiple Subscribers up to their current requested amount or available data?

@akarnokd Thanks! I haven't considered fanout in this scenario, I guess it is questionable whether such a thing would count against returning in a timely manner. Will think about that once I've had some sleep!

akarnokd · 2017-02-13T22:00:05Z

README.md

+| <a name="3.13">13</a>     | While the `Subscription` is not cancelled, `Subscription.cancel()` MUST request the `Publisher` to eventually drop any references to the corresponding subscriber. |
+| [:bulb:](#3.13 "3.13 explained") | *The intent of this rule is to make sure that Subsribers can be properly garbage-collected after their subscription no longer being valid. Re-subscribing with the same Subscriber object is discouraged [see [2.12](#2.12)], but this specification does not mandate that it is disallowed since that would mean having to store previously cancelled subscriptions indefinitely.* |
+| <a name="3.14">14</a>     | While the `Subscription` is not cancelled, calling `Subscription.cancel` MAY cause the `Publisher`, if stateful, to transition into the `shut-down` state if no other `Subscription` exists at this point [see [1.9](#1.9)]. |
+| [:bulb:](#3.14 "3.14 explained") | *The intent of this rule is to allow for Publishers to not accept new Subscribers in response to a cancellation signal.* |


Shouldn't the term "not accept" be explained a bit, i.e., a shut-down Publisher is still expected to call onSubscribe followed by onError or onComplete so the Subscriber is not hung on data that never comes.

@akarnokd Good point. Worth elaborating on!

akarnokd · 2017-02-13T22:03:00Z

README.md

 | <a name="3.15">15</a>     | Calling `Subscription.cancel` MUST return normally. The only legal way to signal failure to a `Subscriber` is via the `onError` method. |
+| [:bulb:](#3.15 "3.15 explained") | *The intent of this rule is to disallow implementations to throw exceptions in response to `cancel` being called, instead directing implementations to signal exceptions through the Subscriber’s `onError`.* |


I'd think cancel means the Subscriber doesn't want any more events, including errors that are caused by the cancellation itself thus a failure in cancel may not go through onError.

@akarnokd Hmm, a failure in cancel doesn't mean that the Subscriber in any way is broken. You raise a good point tho. Question is which behavior is most desirable. Will think about this!

akarnokd · 2017-02-13T22:04:34Z

README.md

-[<a name="footnote-3-2">2</a>] : Avoid heavy computations and other things that would stall the caller´s thread of execution
-
-[<a name="footnote-3-3">3</a>] : As it is not feasibly reachable with current or foreseen hardware within a reasonable amount of time (1 element per nanosecond would take 292 years) to fulfill a demand of 2^63-1, it is allowed for a `Publisher` to stop tracking demand beyond this point.
+| [:bulb:](#3.16 "3.16 explained") | *The intent of this rule is to disallow implementations to throw exceptions in response to `cancel` being called, instead directing implementations to signal exceptions through the Subscriber’s `onError`.* |


Wrong method mentioned?

disallow implementations to throw exceptions in response to request being called

@akarnokd Yes, thank you!

viktorklang · 2017-02-14T10:45:07Z

@reactive-streams/contributors Experimenting with adding a proper glossary section which can be linked to from the various rules: https://github.com/reactive-streams/reactive-streams-jvm/blob/0ceb2ea80683dd4810ecaf168bf85f4430a33c6a/README.md#glossary

ktoso · 2017-02-20T02:03:05Z

README.md

 | <a name="2.3">3</a>       | `Subscriber.onComplete()` and `Subscriber.onError(Throwable t)` MUST NOT call any methods on the `Subscription` or the `Publisher`. |
+| [:bulb:](#2.3 "2.3 explained") | *The intent of this rule is to prevent cycles and race-conditions—between Publisher, Subsription and Subscriber—during the processing of completion signals.* |


~~Keep the same style as elsewhere when referring to Publisher (add `)~~

Ignore this, I just noticed https://github.com/reactive-streams/reactive-streams-jvm/pull/339/files#r101938481

ktoso · 2017-02-20T02:06:08Z

README.md

-| <a name="2.11">11</a>     | A `Subscriber` MUST make sure that all calls on its `onXXX` methods happen-before [[1]](#footnote-2-1) the processing of the respective signals. I.e. the Subscriber must take care of properly publishing the signal to its processing logic. |
+| [:bulb:](#2.10 "2.10 explained") | *The intent of this rule is to establish that Publisher failures may be completely unrelated to signalled demand. This means that Subscribers do not need to poll to find out if the Publisher will not be able to fulfill its requests.* |
+| <a name="2.11">11</a>     | A `Subscriber` MUST make sure that all calls on its `onXXX` methods happen-before the processing of the respective signals. I.e. the Subscriber must take care of properly publishing the signal to its processing logic. |
+| [:bulb:](#2.11 "2.11 explained") | *The intent of this rule is to establish that it is the responsibility of the Subscriber implementation to make sure that asynchronous processing of its signals are thread safe. See [JMM definition of Happen-Before in section 17.4.5](https://docs.oracle.com/javase/specs/jls/se7/html/jls-17.html#jls-17.4.5).* |


The spelling "thread-safe" is used everywhere in the spec elsewhere, use that style.

Link to JDK8? https://docs.oracle.com/javase/specs/jls/se8/html/jls-17.html#jls-17.4.5
Don't have that link for 9 yet..

Hyphenation only happens when the adjective occurs before the noun, not after. So it's "thread-safe signals" but when after "signals are thread safe".

Oh, thanks for clarifying - I did not know this rule. Life is study :)

@viktorklang It should be “happens-before” (note the 's').

@JakeWharton That would be the first time for me to hear about a rule in relation to binding words with a hyphen in English, could you point to the source? (so far and also in conversation with Manning’s copyeditors there were only individual rulings for each specific combination)

Changed to Happens-Before

ktoso · 2017-02-20T02:09:46Z

README.md

-[<a name="footnote-2-1">1</a>] : See JMM definition of Happen-Before in section 17.4.5. on http://docs.oracle.com/javase/specs/jls/se7/html/jls-17.html
+| [:bulb:](#2.12 "2.12 explained") | *The intent of this rule is to establish that it MUST be assumed that the same Subscriber can only be subscribed at most once.* |
+| <a name="2.13">13</a>     | Calling `onSubscribe`, `onNext`, `onError` or `onComplete` MUST [return normally](#term_return_normally) except when any provided parameter is `null` in which case it MUST throw a `java.lang.NullPointerException` to the caller, for all other situations the only legal way for a `Subscriber` to signal failure is by cancelling its `Subscription`. In the case that this rule is violated, any associated `Subscription` to the `Subscriber` MUST be considered as cancelled, and the caller MUST raise this error condition in a fashion that is adequate for the runtime environment. |
+| [:bulb:](#2.13 "2.13 explained") | *The intent of this rule is to establish the semantics for the methods of Subscriber and what the Publisher is allowed to do in which case this rule is violated. By "raising this error condition in a fashion that is adequate for the runtime environment" could mean logging the error or otherwise make someone or something aware of the situation, as the error cannot be signalled to the faulty Subscriber.* |


I noticed now that you don't ``` Publisher and Subscriber etc in the "explained" sections.

That's fine then, ignore my above statement about putting them into ` everywhere then. It's consistent within the explained sections already - LGTM

This seems to be a weird sentence:

By "raising this error ..." could mean

Should it be "Raising this error ..." could mean?

Take a second look :)

ktoso · 2017-02-20T02:12:18Z

README.md

 | <a name="2.9">9</a>       | A `Subscriber` MUST be prepared to receive an `onComplete` signal with or without a preceding `Subscription.request(long n)` call. |
+| [:bulb:](#2.9 "2.9 explained") | *The intent of this rule is to establish that completion is unrelated to the demand flow—this allows for streams which close early, and does not need to "poll for completion".* |


"which complete eagerly, and does not force the Subscriber to "poll for completion"?

complete instead of close, avoiding adding another new word for the same action

The "... and does not need to poll" felt a bit like it's in the wrong tense otherwise; maybe that's just me though hm

Look at my rewording, thanks for finding this!

ktoso · 2017-02-20T02:19:09Z

README.md

-| <a name="3.7">7</a>       | After the `Subscription` is cancelled, additional `Subscription.cancel()` MUST be NOPs. |
+| [:bulb:](#3.2 "3.2 explained") | *The intent of this rule is to make it clear that implementations of `request` must be reentrant, to avoid stack overflows in the case of mutual recursion between `request` and `onNext` (and eventually `onComplete` / `onError`). This implies that Publishers can be `synchronous`, i.e. signalling `onNext`:s on the thread which calls `request`.* |
+| <a name="3.3">3</a>       | `Subscription.request` MUST place an upper bound on possible synchronous recursion between `Publisher` and `Subscriber`. |
+| [:bulb:](#3.3 "3.3 explained") | *The intent of this rule is to complement [see [3.2](#3.2)] by placing an upper limit on the mutual recursion between `request` and `onNext` (and eventually `onComplete` / `onError`). Implementations are RECOMMENDED to limit this mutual recursion to a depth of `1` (ONE)—for the sake of conserving stack space. An example for undesirable synchronous, open recursion would be Subscriber.onNext -> Subscription.request -> Subscriber.onNext -> …, as it very quickly would result in blowing the calling Thread´s stack.* |


"as it very quickly would result" => "as it could result" - is enough, no need to stress it too strongly I think, implementers will decide on their limit.

hanged to "otherwise will"

ktoso

I have to head offline now (on vacation technically speaking for a bit) but will resume reviewing and addressing the specific discussion points in my evening later on (asia time zone). Great additions so far, thanks Viktor.

viktorklang · 2017-02-28T18:27:50Z

@reactive-streams/contributors Everybody, I have now squashed my commits.
Could you please give this PR a second review and a 👍 or 👎? I think this PR addresses a ton of open Issues so it would be great to see it over the finish-line.

THE INTENT OF THIS PR IS TO NOT CHANGE ANY RULE, ONLY TO EXPLAIN THEM

DougLea · 2017-02-28T18:35:44Z

+1: clearly an improvement.

DougLea · 2017-03-01T12:17:51Z

@akarnokd brought up on concurrency-interest list that 3.9 still says that exception is thrown if request is <= 0, not < 0. I thought we once changed this. The jdk9 implementation did (last year), to allow Zero as an OK value to request.

smaldini · 2017-03-01T14:33:50Z

@DougLea @viktorklang I thought as well 3.9 ruled out 0 , especially given the intent is to avoid accounting issues.

ktoso · 2017-03-01T17:05:50Z

👍 on the intent clarifications, nice work Viktor. Thanks for addressing the nitpicks I found.

Let's try to keep this thread focused on the clarifications and continue other debates in new or their-respective issues/PRs - hard to find discussions and chime in otherwise :-)

smaldini · 2017-03-01T17:53:56Z

Yes the intent clarification is good, I misread this 3.9 as something that I thought was here before and wasn't as @DougLea. We can take that on a separate discussion indeed, interestingly the intent description really emphasizes why this is a useless rule at first!

DougLea · 2017-03-01T18:05:49Z

OK, in that case, I guess I am back to complaining about this whole change set! Somehow I missed noticing the zero in new intent. Zero can arise in legitimate scenarios, and in any case has an uncontroversial meaning. Imagine a language spec disallowing addition a+b when one of the arguments happens to be zero.

viktorklang · 2017-03-02T07:34:22Z

I don't see 3.9 changed in this PR so a conversation about changing the rule should be made separately. Sadly, I have no recollection of having changed 3.9 in the past to allow for 0 requesting, does someone have a link that conversation or PR? To me, a request for 0 elements is odd: what semantics would it convey, I mean, if you realize you need 0 elements, the correct course of action is to not request at all? Anyway, let's continue that conversation in some other Issue :)

…

-- Cheers, √

On Mar 1, 2017 7:05 PM, "DougLea" ***@***.***> wrote: OK, in that case, I guess I am back to complaining about this whole change set! Somehow I missed noticing the zero in new intent. Zero can arise in legitimate scenarios, and in any case has an uncontroversial meaning. Imagine a language spec disallowing addition a+b when one of the arguments happens to be zero. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#339 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAqdwpCQPK0W_EQc1PQ2eNevveLcfplks5rhbN-gaJpZM4LvTlO> .

DougLea · 2017-03-02T12:07:51Z

OK, please open elsewhere. I'll also try to dig up previous discussion about why we don't parentally ban things just because they seem odd. Adding zero turns out to be useful sometimes, for example, the base case in a recursive construction.

akarnokd · 2017-03-02T12:31:49Z

@DougLea There is nothing wrong with rule §3.9 in the spec in this regard and in fact it helps internally to know in a deferred requesting scenario that the downstream hasn't requested yet - helping to ensure §1.3 in the process.

Adding zero turns out to be useful sometimes, for example, the base case in a recursive construction.

I've literally written hundreds of Publishers but never came across this as I understand your terminology. Could you elaborate this in a separate issue?

viktorklang · 2017-03-03T00:21:18Z

@reactive-streams/contributors Alright everyone, I'm merging this at the end of this week unless I hear any bad news. :)

viktorklang · 2017-03-05T20:38:12Z

@reactive-streams/contributors Merging!

viktorklang force-pushed the wip-intent-√ branch 2 times, most recently from c15f1c8 to bba4aab Compare January 27, 2017 00:20

viktorklang commented Jan 27, 2017

View reviewed changes

viktorklang changed the title ~~Wip intent √~~ Documentation of intent for each and every rule Feb 10, 2017

akarnokd reviewed Feb 10, 2017

View reviewed changes

akarnokd reviewed Feb 13, 2017

View reviewed changes

viktorklang force-pushed the wip-intent-√ branch from 69fe7a6 to 4ee1058 Compare February 13, 2017 22:19

viktorklang force-pushed the wip-intent-√ branch 4 times, most recently from ae33b0c to 9512a48 Compare February 16, 2017 09:36

ktoso reviewed Feb 20, 2017

View reviewed changes

Adds a Glossary, Intent-sections and harmonizes verbiage

b755786

viktorklang force-pushed the wip-intent-√ branch from 8227bde to b755786 Compare February 28, 2017 18:26

viktorklang merged commit 93874e7 into master Mar 5, 2017

akarnokd mentioned this pull request Mar 5, 2017

Minor rewording of 2.6 to make it easier to understand. #342

Merged

viktorklang deleted the wip-intent-√ branch June 29, 2017 09:11

		\| <a name="2.12">12</a> \| `Subscriber.onSubscribe` MUST be called at most once for a given `Subscriber` (based on object equality). \|
		\| [:bulb:](#2.12 "2.12 explained") \| The intent of this rule is to establish that it is not to be considered intended behavior that the same Subscriber is concurrently subscribed to several Publishers. \|

		\| <a name="3.10">10</a> \| While the `Subscription` is not cancelled, `Subscription.request(long n)` MAY synchronously call `onNext` on this (or other) subscriber(s). \|
		\| [:bulb:](#3.10 "3.10 explained") \| The intent of this rule is to establish that it is allowed to create synchronou Publishers, i.e. Publishers who execute their logic on the calling thread. \|

		\| <a name="3.15">15</a> \| Calling `Subscription.cancel` MUST return normally. The only legal way to signal failure to a `Subscriber` is via the `onError` method. \|
		\| [:bulb:](#3.15 "3.15 explained") \| The intent of this rule is to disallow implementations to throw exceptions in response to `cancel` being called, instead directing implementations to signal exceptions through the Subscriber’s `onError`. \|

		\| <a name="2.3">3</a> \| `Subscriber.onComplete()` and `Subscriber.onError(Throwable t)` MUST NOT call any methods on the `Subscription` or the `Publisher`. \|
		\| [:bulb:](#2.3 "2.3 explained") \| The intent of this rule is to prevent cycles and race-conditions—between Publisher, Subsription and Subscriber—during the processing of completion signals. \|

		\| <a name="2.9">9</a> \| A `Subscriber` MUST be prepared to receive an `onComplete` signal with or without a preceding `Subscription.request(long n)` call. \|
		\| [:bulb:](#2.9 "2.9 explained") \| The intent of this rule is to establish that completion is unrelated to the demand flow—this allows for streams which close early, and does not need to "poll for completion". \|

Documentation of intent for each and every rule #339

Documentation of intent for each and every rule #339

Conversation

viktorklang commented Jan 27, 2017

Choose a reason for hiding this comment

viktorklang commented Feb 10, 2017

viktorklang commented Feb 10, 2017

DougLea commented Feb 10, 2017

smaldini commented Feb 10, 2017

viktorklang commented Feb 10, 2017

akarnokd commented Feb 10, 2017 • edited Loading

§1.3: onNext should not be called concurrently until onSubscribe returns

§2.3: onError or onComplete must not call cancel/request

§3.9: negative requests should emit an onError(IllegalArgumentException)

§2.12: call onSubscribe once per Subscriber instance

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smaldini commented Feb 10, 2017 • edited Loading

DougLea commented Feb 10, 2017

viktorklang commented Feb 13, 2017

akarnokd commented Feb 13, 2017

viktorklang commented Feb 13, 2017

akarnokd commented Feb 13, 2017

smaldini commented Feb 13, 2017 • edited Loading

viktorklang commented Feb 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

viktorklang commented Feb 14, 2017

ktoso Feb 20, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ktoso Feb 20, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ktoso Feb 20, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ktoso Feb 20, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ktoso left a comment

Choose a reason for hiding this comment

viktorklang commented Feb 28, 2017

DougLea commented Feb 28, 2017

DougLea commented Mar 1, 2017

smaldini commented Mar 1, 2017

ktoso commented Mar 1, 2017

smaldini commented Mar 1, 2017

DougLea commented Mar 1, 2017

viktorklang commented Mar 2, 2017 via email

DougLea commented Mar 2, 2017

akarnokd commented Mar 2, 2017

viktorklang commented Mar 3, 2017

viktorklang commented Mar 5, 2017

akarnokd commented Feb 10, 2017 •

edited

Loading

§1.3: `onNext` should not be called concurrently until `onSubscribe` returns

§2.12: call `onSubscribe` once per `Subscriber` instance

smaldini commented Feb 10, 2017 •

edited

Loading

smaldini commented Feb 13, 2017 •

edited

Loading

ktoso Feb 20, 2017 •

edited

Loading

ktoso Feb 20, 2017 •

edited

Loading

ktoso Feb 20, 2017 •

edited

Loading

ktoso Feb 20, 2017 •

edited

Loading