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

Subscription.cancel MUST BE thread-safe #533

Open
tkountis opened this issue Jun 23, 2021 · 4 comments
Open

Subscription.cancel MUST BE thread-safe #533

tkountis opened this issue Jun 23, 2021 · 4 comments

Comments

@tkountis
Copy link

Rule 3.5

Subscription.cancel MUST respect the responsivity of its caller by returning in a timely manner, MUST be idempotent and MUST be thread-safe.

mentions thread-safety and also links the keyword to it's definition in this context, which mentions "async".

Can be safely invoked synchronously, or asychronously, without requiring external synchronization to ensure program correctness.

The rule seems to contradict other rules, like:

  • 2.7

A Subscriber MUST ensure that all calls on its Subscription's request and cancel methods are performed serially.

  • 3.1

Subscription.request and Subscription.cancel MUST only be called inside of its Subscriber context.

And lastly the "MUST be thread-safe" doesn't equally apply to 'Subscription.request'.
Is this obsolete, or is my understanding wrong?

@akarnokd
Copy link
Contributor

The definition of serially should clear things up.

Serial(ly) In the context of a 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.

In practice, implementations of the Subscription methods end up thread-safe anyway.

@tkountis
Copy link
Author

Thanks for the prompt response 👍
I do understand the 'serially' part, however the fact that Subscription methods end up thread-safe is not the point. My argument is, that the spec defines that cancel must be 'thread-safe', which isn't necessary true, likewise for request which doesn't list this clause. Shouldn't this part be removed or re-worded for clarity?

@NiteshKant
Copy link

The inconsistency between cancel() and request() does seem questionable.

@NiteshKant
Copy link

In practice, implementations of the Subscription methods end up thread-safe anyway.

Not sure if this is generally true as it depends on how a Publisher is using a Subscription instance.
As an example this Subscription instance usage is such that an instance is only used by the associated Subscriber and hence can assume serial access as required by rule 2.7

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