Commit 24f35af
Nitesh Kant
Clarify intent of rule 1.9
Rule 1.9 defines that `onSubscribe()` on a `Subscriber` should be called before calling any other method on that `Subscriber`. However, this does not clarify that if `onSubscribe()` is signalled asynchronously then there should be a happens-before relationship between `subscribe()` and signalling of `onSubscribe()`.
In absence of such a guarantee, non-final fields initialized in a constructor have no guarantee to be visible inside `onSubscribe()`.
Considering a simple example:
public class UnsafeSubscriber implements Subscriber<String> {
private boolean duplicateOnSubscribe = false;
@OverRide
public void onSubscribe(final Subscription s) {
if (duplicateOnSubscribe) {
throw new IllegalStateException("Duplicate onSubscribe() calls.");
}
duplicateOnSubscribe = true;
}
@OverRide
public void onNext(final String s) {
}
@OverRide
public void onError(final Throwable t) {
}
@OverRide
public void onComplete() {
}
}
If an UnsafeSubscriber instance is created in a different thread than the one that invokes onSubscribe() (true for an asynchronous Publisher), according to the java memory model, this statement inside onSubscribe():
if (duplicateOnSubscribe) {
is guaranteed to compute to false if and only if the instance is published safely between these threads. None of the rules in the specifications establish a happens-before relationship between Publisher#subscribe() and Subscriber#onSubscribe(). So, the usage above can be categorized as unsafe. In a more convoluted form, the assignment:
private boolean duplicateOnSubscribe = false;
can be interleaved with
duplicateOnSubscribe = true; such that duplicateOnSubscribe is set to false later.
Has this been considered before or am I missing something?
Fixes reactive-streams#4861 parent de7ad73 commit 24f35af
2 files changed
+2
-1
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
47 | 47 | | |
48 | 48 | | |
49 | 49 | | |
| 50 | + | |
50 | 51 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
111 | 111 | | |
112 | 112 | | |
113 | 113 | | |
114 | | - | |
| 114 | + | |
115 | 115 | | |
116 | 116 | | |
117 | 117 | | |
| |||
0 commit comments