Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Race condition in MonoCollect leads to NullPointerException #2186

Closed
rstoyanchev opened this issue Jun 10, 2020 · 0 comments · Fixed by #2193
Closed

Race condition in MonoCollect leads to NullPointerException #2186

rstoyanchev opened this issue Jun 10, 2020 · 0 comments · Fixed by #2193
Assignees
Labels
type/bug A general bug

Comments

@rstoyanchev
Copy link
Contributor

The example in spring-projects/spring-framework#25216 shows a NullPointerException in the logs:

java.lang.NullPointerException: null
at reactor.core.publisher.MonoCollect$CollectSubscriber.onNext(MonoCollect.java:124) [reactor-core-3.3.6.RELEASE.jar:3.3.6.RELEASE]
at reactor.core.publisher.FluxMap$MapSubscriber.onNext(FluxMap.java:114) [reactor-core-3.3.6.RELEASE.jar:3.3.6.RELEASE]
at reactor.core.publisher.FluxPeek$PeekSubscriber.onNext(FluxPeek.java:192) [reactor-core-3.3.6.RELEASE.jar:3.3.6.RELEASE]
at reactor.core.publisher.FluxPeek$PeekSubscriber.onNext(FluxPeek.java:192) [reactor-core-3.3.6.RELEASE.jar:3.3.6.RELEASE]
at reactor.core.publisher.FluxMap$MapSubscriber.onNext(FluxMap.java:114) [reactor-core-3.3.6.RELEASE.jar:3.3.6.RELEASE]
at reactor.netty.channel.FluxReceive.onInboundNext(FluxReceive.java:330) [reactor-netty-0.9.8.RELEASE.jar:0.9.8.RELEASE]
...

This happens while collecting buffers in DataBufferUtils#join and what's null is the value field (the container collecting the items) and not the action. In a debugger this is where the NPE is raised:

at org.springframework.core.io.buffer.DataBufferUtils$$Lambda$340.551556520.accept(Unknown Source:-1)
at reactor.core.publisher.MonoCollect$CollectSubscriber.onNext(MonoCollect.java:124)
at reactor.core.publisher.FluxMap$MapSubscriber.onNext(FluxMap.java:114)
at reactor.core.publisher.FluxPeek$PeekSubscriber.onNext(FluxPeek.java:192)
at reactor.core.publisher.FluxPeek$PeekSubscriber.onNext(FluxPeek.java:192)
at reactor.core.publisher.FluxMap$MapSubscriber.onNext(FluxMap.java:114)
at reactor.netty.channel.FluxReceive.onInboundNext(FluxReceive.java:330)

As far as I can see in MonoCollect around line 124 the value field is passed in as-is but it can be set to null in Operators around line 1728 in the cancel method of MonoSubscriber. Probably the code should look more like what is in MonoCollectList with a local variable and a null check.

@reactorbot reactorbot added the ❓need-triage This issue needs triage, hasn't been looked at by a team member yet label Jun 10, 2020
@simonbasle simonbasle added type/bug A general bug and removed ❓need-triage This issue needs triage, hasn't been looked at by a team member yet labels Jun 11, 2020
@simonbasle simonbasle added this to the 3.2.19.RELEASE milestone Jun 11, 2020
rstoyanchev added a commit to rstoyanchev/reactor-core that referenced this issue Jun 16, 2020
This change aligns the implementation of MonoCollect to be comparable
to MonoList. Effectively they're the same, a MonoList is but a
MonoCollect with an ArrayList as the container.

Fix reactor#2186
@rstoyanchev rstoyanchev linked a pull request Jun 16, 2020 that will close this issue
rstoyanchev added a commit to rstoyanchev/reactor-core that referenced this issue Jun 16, 2020
This change aligns the implementation of MonoCollect to be comparable
to MonoList. Effectively they're the same, a MonoList is but a
MonoCollect with an ArrayList as the container.

Fix reactor#2186
simonbasle added a commit that referenced this issue Jun 16, 2020
simonbasle added a commit that referenced this issue Jun 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug A general bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants