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

[Reactive] Workaround for an RS 1.0.3 TCK bug #1568

Merged
merged 4 commits into from
Mar 24, 2020

Conversation

akarnokd
Copy link
Collaborator

Cross cancellation from an onError/onComplete method is not correctly verified by the Reactive Streams TCK. There is a patch waiting to be included but it could take an arbitrary amount of time to get it in a released form.

In addition, the original workaround of using the ForkJoinPool sometimes doesn't work because the FJP executes the task on the caller thread, thus triggering the TCK bug again.

This PR adds a workaround in the form of a configurable ExecutorService which is set to a single-threaded one for the duration of the TCK.

@danielkec danielkec self-assigned this Mar 23, 2020
@danielkec danielkec added the reactive Reactive streams and related components label Mar 23, 2020
@danielkec danielkec added this to the 2.0.0 milestone Mar 23, 2020
@romain-grecourt
Copy link
Contributor

/oca-checked

Comment on lines +541 to +542
// Workaround for a TCK bug when calling cancel() from any method named onComplete().
private static volatile ExecutorService coupledExecutor = ForkJoinPool.commonPool();
Copy link
Contributor

Choose a reason for hiding this comment

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

For future reference TCK fix: reactive-streams/reactive-streams-jvm#483

@danielkec danielkec merged commit 3099935 into helidon-io:master Mar 24, 2020
@akarnokd akarnokd deleted the MPRSWorkaround branch March 24, 2020 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
reactive Reactive streams and related components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants