NPE in a simple project Spring Boot (Servlet) + RSocket + Sleuth

[SimSonic]

Describe the bug
There is a NPE in a simple application based on Spring Boot 2.3.5.RELEASE or 2.3.6.RELEASE with RSocket and Sleuth in dependencies.
The problem arises when trying to execute request-response from client to the server.

Caused by: java.lang.NullPointerException: null
	at io.rsocket.core.RequestOperator.currentContext(RequestOperator.java:89) ~[rsocket-core-1.0.3.jar:na]
	at reactor.core.publisher.Operators.reportThrowInSubscribe(Operators.java:230) ~[reactor-core-3.3.11.RELEASE.jar:3.3.11.RELEASE]
	at reactor.core.publisher.InternalMonoOperator.subscribe(InternalMonoOperator.java:71) ~[reactor-core-3.3.11.RELEASE.jar:3.3.11.RELEASE]
	at io.rsocket.core.RequestOperator.<init>(RequestOperator.java:42) ~[rsocket-core-1.0.3.jar:na]
	at io.rsocket.core.RSocketRequester$1.<init>(RSocketRequester.java:271) ~[rsocket-core-1.0.3.jar:na]
	at io.rsocket.core.RSocketRequester.handleRequestResponse(RSocketRequester.java:271) ~[rsocket-core-1.0.3.jar:na]
	at io.rsocket.core.RSocketRequester.requestResponse(RSocketRequester.java:164) ~[rsocket-core-1.0.3.jar:na]
	at reactor.core.publisher.MonoFlatMap$FlatMapMain.onNext(MonoFlatMap.java:118) ~[reactor-core-3.3.11.RELEASE.jar:3.3.11.RELEASE]
	at reactor.core.publisher.FluxContextStart$ContextStartSubscriber.onNext(FluxContextStart.java:96) ~[reactor-core-3.3.11.RELEASE.jar:3.3.11.RELEASE]
	at reactor.core.publisher.FluxContextStart$ContextStartSubscriber.onNext(FluxContextStart.java:96) ~[reactor-core-3.3.11.RELEASE.jar:3.3.11.RELEASE]
	at reactor.core.publisher.FluxMap$MapConditionalSubscriber.onNext(FluxMap.java:213) ~[reactor-core-3.3.11.RELEASE.jar:3.3.11.RELEASE]
	at reactor.core.publisher.Operators$MonoSubscriber.complete(Operators.java:1782) ~[reactor-core-3.3.11.RELEASE.jar:3.3.11.RELEASE]
	at reactor.core.publisher.MonoZip$ZipCoordinator.signal(MonoZip.java:247) ~[reactor-core-3.3.11.RELEASE.jar:3.3.11.RELEASE]
	at reactor.core.publisher.MonoZip$ZipInner.onNext(MonoZip.java:329) ~[reactor-core-3.3.11.RELEASE.jar:3.3.11.RELEASE]
	at reactor.core.publisher.Operators$MonoSubscriber.complete(Operators.java:1782) ~[reactor-core-3.3.11.RELEASE.jar:3.3.11.RELEASE]
	at reactor.core.publisher.MonoCallable.subscribe(MonoCallable.java:61) ~[reactor-core-3.3.11.RELEASE.jar:3.3.11.RELEASE]
	at reactor.core.publisher.Mono.subscribe(Mono.java:4213) ~[reactor-core-3.3.11.RELEASE.jar:3.3.11.RELEASE]
	at reactor.core.publisher.MonoZip.subscribe(MonoZip.java:128) ~[reactor-core-3.3.11.RELEASE.jar:3.3.11.RELEASE]
	at reactor.core.publisher.Mono.subscribe(Mono.java:4213) ~[reactor-core-3.3.11.RELEASE.jar:3.3.11.RELEASE]
	at reactor.core.publisher.Mono.block(Mono.java:1679) ~[reactor-core-3.3.11.RELEASE.jar:3.3.11.RELEASE]
	at example.ClientMain.run(ClientMain.java:32) ~[classes/:na]
	at org.springframework.boot.SpringApplication.callRunner(SpringApplication.java:795) ~[spring-boot-2.3.6.RELEASE.jar:2.3.6.RELEASE]
	... 3 common frames omitted

Sample
spring-boot-235-rsocket-npe-example.zip
Attached zip containes Maven multi-module .pom with both server and client. To reproduce the bug just start server and then client.

Other info
Switching Spring Cloud down from Hoxton.SR9 to Hoxton.SR8 doesn't solve the issue.

Switching to Spring Boot 2.3.4.RELEASE fixes the issue. Upgrade to Spring Boot 2.4.0 also fixes it.

[marcingrzejszczak]

I don't see anything Sleuth related here. When you disable / remove sleuth things work fine?

[SimSonic]

@marcingrzejszczak Yes, just comment it out from pom.xml and all works.

Specifically Sleuth is present in the debugger:

[marcingrzejszczak]

pinging @violetagg, @simonbasle

The interesting thing is that most likely it's a bug in RSocket (or maybe I'm doing stuff to early in Sleuth).

public static <T> Function<? super Publisher<T>, ? extends Publisher<T>> scopePassingSpanOperator(
			ConfigurableApplicationContext springContext) {
		LazyBean<CurrentTraceContext> lazyCurrentTraceContext = LazyBean
				.create(springContext, CurrentTraceContext.class);
		return Operators.liftPublisher((p, sub) -> {
			if (p instanceof Fuseable.ScalarCallable) {
				return sub;
			}

			...

			// THIS LINE FAILS. [sub] is not null, however it's calling RequestOperator's [currentContext] which in effect is calling [this.actual.currentContext();] where [this.actual] is null
			Context context = sub.currentContext();

I can of course do a try catch on trying to retrieve the current context however what is the case where RequestOperator.actual can be null ? Is it a valid situation? Shouldn't they actually take care of the actual being null ?

[simonbasle]

maybe @OlegDokuka can tell us more about the RequestOperator. At a minimum, if it defers storing its Subscriber somehow (which is unusual at best), its currentContext() method should not return null but return actual == null ? Context.empty() : actual.currentContext().

One thing though: some operators may change Context at a later point, which means that "caching" the first Context you see from downstream is generally not a great idea. I wonder if that's the case for Sleuth?

For instance, imagine Sleuth tries to read the Context from the RequestOperator, but at that point actual is not yet defined (for whatever reason). Instead of returning null, let's pretend it now returns Context.empty(). If Sleuth captures that empty context and never calls requestOperator.currentContext() again, then it'll miss the fact that RequestOperator get an actual later on (again, assuming this is a specific behavior of RequestOperator. there could be a simpler explanation for the lack of actual in there)

[marcingrzejszczak]

maybe @OlegDokuka can tell us more about the RequestOperator. At a minimum, if it defers storing its Subscriber somehow (which is unusual at best), its currentContext() method should not return null.

Until there's a fix in RSocket I think I'll just add guards to the existing code and fallback to an empty context.

One thing though: some operators may change Context at a later point, which means that "caching" the first Context you see from downstream is generally not a great idea. I wonder if that's the case for Sleuth?

We're reusing the context I don't think we're caching it.