Reactor-core: Mono onComplete order is confusing

[martin-tarjanyi]

Expected behavior

Following code:

Mono.just("outer")
    .then(Mono.just("inner").doOnTerminate(() -> System.out.println("Completed inner mono.")))
    .doOnTerminate(() -> System.out.println("Completed outer mono."))
    .subscribe(System.out::println, Throwable::printStackTrace, () -> System.out.println("Subscriber onComplete called."));

Expected output:
_inner
Completed inner mono.
Completed outer mono.
Subscriber onComplete called._

Actual behavior

Actual output:
_inner
Subscriber onComplete called.
Complete outer mono.
Complete inner mono._

Reactor Core version

3.2.8

Additional details

Switching the inner Mono to an empty one produces the expected output

Mono.just("outer")
    .then(Mono.empty().doOnTerminate(() -> System.out.println("Completed inner mono.")))
    .doOnTerminate(() -> System.out.println("Completed outer mono."))
    .subscribe(System.out::println, Throwable::printStackTrace, () -> System.out.println("Subscriber onComplete called."));

Output:
_Completed inner mono.
Completed outer mono.
Subscriber onComplete called._

Switching to thenMany(Flux) instead of then(Mono) also creates the expected output.

Looking at a more basic example:

Mono.just("outer")
    .doOnTerminate(() -> System.out.println("Complete outer mono."))
    .subscribe(System.out::println, Throwable::printStackTrace, () -> System.out.println("Subscriber onComplete called."));

also produces slightly unexpected output:
_outer
Subscriber onComplete called.
Complete outer mono._

Switching to Flux here produces the expected output.
For the first example the issue is somewhere inside MonoIgnoreThen I believe.

[ZhangSanFengByGit]

Doc

 * Let this {@link Mono} complete then play another Mono.
 * In other words ignore element from this {@link Mono} and transform its completion signal into the
 * emission and completion signal of a provided {@code Mono<V>}. Error signal is
 * replayed in the resulting {@code Mono<V>}.

So

The first example of yours is actually function well. The tricky thing here is just the sequence of happening of each operators here:
Main Mono completes(print function in subscriber functions) -> Main Mono doOnTerminate() -> (the previous step as complete signal to inner Mono) inner Mono completes -> Inner Mono doOnTerminate()

[smaldini]

This is an interesting behavior you noticed here:

Mono.subscribe will itself call onComplete on onNext, due to the Mono nature itself (one transition at most). In fact some other operators will take the liberty of not waiting for an onComplete signal if they have received the onNext. The idea is to avoid traversing twice the operators with an onNext and an onComplete, so Mono should not even call onComplete (interested operators will produce it anyway).

There might be a need to review that path again to make sure we have applied that optimization all the way and avoid confusing result. I am marking that issue for more exploration in the current 3.3 given it might lead to a behavior change.

[simonbasle]

Clarifying action items: we need to make sure all doOn operators perform the same shortcut of triggering downstream's onComplete immediately in the onNext path, that is to say:

1. trigger any onNext side effect
2. set a boolean done flag to true
3. trigger any onComplete side effect
4. propagate onNext to actual
5. propagate onComplete to actual
6. make sure the onComplete() method checks done and does not apply onComplete side effects twice due to the above

[simonbasle]

I'm doubting more and more the above plan. doOn* operators should probably not trigger the early onComplete themselves. Source monos, or monos that adapt a generic Publisher should definitely, but provided we're sure to deal with a Mono source, a "passthrough" operator shouldn't.

Isn't the problem instead coming from LambdaMonoSubscriber triggering the early onComplete? Btw as long as this is the case, "inner" and " Subscriber onComplete called" above will always be clumped together.

[simonbasle]

eg. even applying the plan above just moves of the 2 first lines at the end, resulting in an IMHO even more confusing:

Completed inner mono.
Completed outer mono.
inner
Subscriber onComplete called.

If you add doOnNext before each doOnTerminate, things get interesting as well:

Emitted inner mono
Completed inner mono.
Emitted outer mono
Completed outer mono.
inner
Subscriber onComplete called.