Typos and Rule 3.5 fixes #448
Conversation
|
@OlegDokuka Please add yourself in the CopyrightWaivers.txt in this PR. |
README.md
Outdated
| @@ -142,7 +142,7 @@ public interface Subscriber<T> { | |||
| | <a name="2.6">6</a> | A `Subscriber` MUST call `Subscription.cancel()` if the `Subscription` is no longer needed. | | |||
| | [:bulb:](#2.6 "2.6 explained") | *The intent of this rule is to establish that Subscribers cannot just throw Subscriptions away when they are no longer needed, they have to call `cancel` so that resources held by that Subscription can be safely, and timely, reclaimed. An example of this would be a Subscriber which is only interested in a specific element, which would then cancel its Subscription to signal its completion to the Publisher.* | | |||
| | <a name="2.7">7</a> | A Subscriber MUST ensure that all calls on its Subscription's request and cancel methods are performed [serially](#term_serially). | | |||
| | [:bulb:](#2.7 "2.7 explained") | *The intent of this rule is to permit the calling of the request and cancel methods (including from multiple threads) if and only if a happens-before relation between each of the calls is established..* | | |||
| | [:bulb:](#2.7 "2.7 explained") | *The intent of this rule is to permit the calling of the request and cancel methods (including from multiple threads) if and only if a happens-before relation between each of the calls is established.* | | |||
README.md
Outdated
| @@ -175,7 +175,7 @@ public interface Subscription { | |||
| | [: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 otherwise will result in blowing the calling thread´s stack.* | | |||
| | <a name="3.4">4</a> | `Subscription.request` SHOULD respect the responsivity of its caller by returning in a timely manner. | | |||
| | [:bulb:](#3.4 "3.4 explained") | *The intent of this rule is to establish that `request` is intended to be a [non-obstructing](#term_non-obstructing) method, and should be as quick to execute as possible on the calling thread, so avoid heavy computations and other things that would stall the caller´s thread of execution.* | | |||
| | <a name="3.5">5</a> | `Subscription.cancel` MUST respect the responsivity of its caller by returning in a timely manner, MUST be idempotent and MUST be [thread-safe](#term_thread-safe). | | |||
| | <a name="3.5">5</a> | `Subscription.cancel` MUST respect the responsivity of its caller by returning in a timely manner, MUST be idempotent and MUST be executed [serially](#term_serially). | | |||
There was a problem hiding this comment.
@davidmoten @reactive-streams/contributors @OlegDokuka
Should this read: MUST be executed [serially](#term_serially)?
There was a problem hiding this comment.
Ok, now I'm thinking that is redundant since it overlaps with 2.7
There was a problem hiding this comment.
fixed. Removed and MUST be executed [serially](#term_serially) since it is the responsibility of Subscriber to do make call serially on Subscription
There was a problem hiding this comment.
No, do not remove that sentence please.
There was a problem hiding this comment.
ok, then we need to rever back thread-safe notion
There was a problem hiding this comment.
but it will contradict with serial access because thread-safety does not require external sync where 2.7 states that external sync is needed on calling cancel. Thus it seems that serial access needed only for Subscription#request (if that was an intent of that rule before)
There was a problem hiding this comment.
In general, the 2.7 had sounded like access to your stored subscription in the way you get access to the field from any thread without a problem. Thus external synchronization was mentioned in order to clarify that calling to subscription should be with happens before. It did not say that calling to methods must be done serially
There was a problem hiding this comment.
To convey that the implementation of cancel needs to be thread safe, the following verbiage should work: "and MUST be implemented to execute serially"?
Update CopyrightWaivers.txt
|
@viktorklang we should ensure that all Reactive Libs vendors are aware of those changes in the spec since the current implementation of libraries implemented differently and all external synchronization is done within Subscription itself. |
|
@OlegDokuka The changes are not changes in semantics, only clarification of message/expectations. |
| | <a name="term_terminal_state">Terminal state</a> | For a Publisher: When `onComplete` or `onError` has been signalled. For a Subscriber: When an `onComplete` or `onError` has been received.| | ||
| | <a name="term_nop">NOP</a> | Execution that has no detectable effect to the calling thread, and can as such safely be called any number of times.| | ||
| | <a name="term_serially">Serial(ly)</a> | In the context of a [Signal](#term_signal), non-overlapping. In the context of the JVM, calls to methods on an object are serial if and only if there is a happens-before relationship between those calls (implying also that the calls do not overlap). When the calls are performed asynchronously, coordination to establish the happens-before relationship is to be implemented using techniques such as, but not limited to, atomics, monitors, or locks. | | ||
| | <a name="term_thread-safe">Thread-safe</a> | Can be safely invoked synchronously, or asychronously, without requiring external synchronization to ensure program correctness. | |
There was a problem hiding this comment.
@viktorklang rolled back tread-safe notion back since it is used in 3.5
There was a problem hiding this comment.
@OlegDokuka This diff seems worryingly large, it should be possible to just reintroduce the thread-safe term and keep the other bits intact. Could you squash the commits and see what the diff looks like?
|
@viktorklang I reread the description of |
README.md
Outdated
| @@ -276,7 +276,6 @@ Queue bounds can be controlled by a subscriber signaling demand for the appropri | |||
|
|
|||
| Then the maximum number of elements that may arrive—until more demand is signaled to the Publisher—is `P - N`. In the case that the subscriber also knows the number of elements B in its input buffer then this bound can be refined to `P - B - N`. | |||
|
|
|||
| These bounds must be respected by a publisher independent of whether the source it represents can be backpressured or not. In the case of sources whose production rate cannot be influenced—for example clock ticks or mouse movement—the publisher must choose to either buffer or drop elements to obey the imposed bounds. | |||
|
closes in favour of #449 |
No description provided.