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

repeatWhenEmpty hangs on cancel when discard hook is present #2196

Closed
chringwer opened this issue Jun 17, 2020 · 1 comment · Fixed by #2216
Closed

repeatWhenEmpty hangs on cancel when discard hook is present #2196

chringwer opened this issue Jun 17, 2020 · 1 comment · Fixed by #2216
Assignees
Labels
good first issue Ideal for a new contributor, we'll help help wanted We need contributions on this type/bug A general bug

Comments

@chringwer
Copy link

Expected Behavior

It should be safe to cancel the sequence.

Actual Behavior

Cancelling a sequence which is in a repeatWhenEmpty() loop will cause issues when a discard hook has been registered. In that case Operators.onDiscardMultiple() is invoked with the iterator used to keep track of the current iteration and the hook would be called for every remaining number up to Long.MAX_VALUE.

Steps to Reproduce

This test case will succeed when removing the doOnDiscard() operator.

@Test
public void shouldCompleteEventually()
{
    StepVerifier.create(Mono.empty()
                            .repeatWhenEmpty(Repeat.once())
                            .doOnDiscard(PooledDataBuffer.class, DataBufferUtils::release))
                .thenAwait()
                .thenCancel()
                .verify(Duration.ofSeconds(5));
}

Possible Solution

Probably we should treat the remaining iterations in a way that they can be safely discarded without bothering any application level hooks. Also, I guess FluxStream should not be considered "knownToBeFinite" if its size is Long.MAX_VALUE.

Your Environment

  • Reactor version(s) used: 3.3.6.RELEASE
@reactorbot reactorbot added the ❓need-triage This issue needs triage, hasn't been looked at by a team member yet label Jun 17, 2020
@simonbasle simonbasle added good first issue Ideal for a new contributor, we'll help help wanted We need contributions on this 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 19, 2020
@simonbasle simonbasle added this to the 3.2.19.RELEASE milestone Jun 19, 2020
@simonbasle
Copy link
Contributor

The operator is an alias, it could use .index() rather than a Stream for its purpose, which would eliminate the issue (see #2014 for context of the iteration)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Ideal for a new contributor, we'll help help wanted We need contributions on this type/bug A general bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants