reactor-core: Map and flatMap operator chaining is not stack-safe
Expected behavior
Chaining large numbers of map and flatMap operators is stack-safe
Actual behavior
StackOverflow on MonoMapFuseable.subscribe
Steps to reproduce
Mono<Integer> initial = Mono.just(0);
for (int i = 0; i < 5000; i++) {
int currentI = i;
initial = initial.map(previous -> currentI);
}
assert initial.block() == 5000;
Yields a StackOverflow error:
java.lang.StackOverflowError
at reactor.core.publisher.MonoMapFuseable.subscribe(MonoMapFuseable.java)
at reactor.core.publisher.MonoMapFuseable.subscribe(MonoMapFuseable.java:59)
at reactor.core.publisher.MonoMapFuseable.subscribe(MonoMapFuseable.java:59)
at reactor.core.publisher.MonoMapFuseable.subscribe(MonoMapFuseable.java:59)
at reactor.core.publisher.MonoMapFuseable.subscribe(MonoMapFuseable.java:59)
at reactor.core.publisher.MonoMapFuseable.subscribe(MonoMapFuseable.java:59)
at reactor.core.publisher.MonoMapFuseable.subscribe(MonoMapFuseable.java:59)
...
Reactor Core version
3.1.8.RELEASE
JVM version (e.g. java -version
)
1.7 with Kotlin in the project, 1.8 in the system
About this issue
- Original URL
- State: closed
- Created 6 years ago
- Reactions: 1
- Comments: 15 (8 by maintainers)
Commits related to this issue
- WIP #1441 Introduce trampolining in deep chains of operators — committed to reactor/reactor-core by simonbasle 5 years ago
- WIP #1441 Introduce stack safety by trampolining deep operator chains Introducing the Stacksafe util in Operators. — committed to reactor/reactor-core by simonbasle 5 years ago
- WIP #1441 Introduce stack safety by trampolining deep operator chains Introducing the Stacksafe util in Operators. — committed to reactor/reactor-core by simonbasle 5 years ago
It could be a trade-off that could affect day-to-day Flux/Mono use (I think its around 10-30 operators for a given use case/request): we could always check via instanceof the current operator to see if its the same type and conflate the function via “Function.andThen”. That would be assembly-time fusion and it would create a single map/flatMap as we do when we chain multiple concatWith, mergeWith and zipWith.
Note that if we generalize this pattern its something to keep in mind as well for a future generation of reactor architecture.
I’m going to close this one unfortunately, as the potential for side effects, subtle bugs down the road and general maintainability hit outweights the benefits in my opinion. Keeping this in a corner of our head in case there’s a heavy rearchitecturing of Reactor in the future, though.
IIRC, and @nomisRev can help me here, what we’ve done in Arrow is trampoline every N operators to reset the stack.