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

ISPN-128806 BlockingManagerImpl#thenRunBlocking doesn't join #9100

Conversation

wburns
Copy link
Member

@wburns wburns commented Mar 5, 2021

@@ -197,6 +197,7 @@ public void execute(Runnable command) {
log.tracef("Invoked thenRun on a blocking thread, joining %s in same blocking thread", traceId);
}
try {
CompletionStages.join(stage);
return stage.thenRun(runnable);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it isn't trivial for me xD
Why do you need stage.thenRun(runnable)?
Could you just invoke runnable.run() and return CompletableFutures.completedNull()?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, either way would work. The point is the stage needs to be complete. I can change it to that if you want.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say to invoke the Runnable directly.
java.util.concurrent.CompletableFuture#thenRun() seems to create a new CompletableFuture instance.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I already pushed the change. 👍

@wburns wburns force-pushed the ISPN-12806_BlockingManager_thenRunBlocking branch from e0b75e6 to cd2f763 Compare March 5, 2021 16:03
@ryanemerson ryanemerson merged commit e62b33c into infinispan:master Mar 10, 2021
@ryanemerson
Copy link
Contributor

Thanks @wburns

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants