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

Most upvoted comments

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.