=tck #427 TCK should stress subscriber rule 2.5 in concurrent setting #428
Conversation
| @@ -231,6 +234,77 @@ public String toString() { | |||
| }}; | |||
| } | |||
|
|
|||
| @Override @Test | |||
| public void required_spec205_blackbox_mustCallSubscriptionCancelIfItAlreadyHasAnSubscriptionAndReceivesAnotherOnSubscribeSignalConcurrently() throws Throwable { | |||
There was a problem hiding this comment.
Travis may not have the proper concurrency to verify this. I'm constantly facing the problem testing race conditions in RxJava requiring two dedicated cores running practically at the same time to trigger small windows around CAS instructions.
This could be the reason the build failed: https://travis-ci.org/reactive-streams/reactive-streams-jvm/jobs/352388267#L1483
There was a problem hiding this comment.
Yeah, quite likely it'll be a pain to have travis execute this as intended. Let's discuss on the ticket first though if this PR makes sense or not, I'm fine with just dropping if if we agree this simply rides on the Publisher's guarnatees spec-wise
| invokeOnSubscribeConcurrentlyTasks.add(invokeOnSubscribe); | ||
| } | ||
|
|
||
| List<Future<Void>> futures = Executors.newFixedThreadPool(16).invokeAll(invokeOnSubscribeConcurrentlyTasks); |
There was a problem hiding this comment.
Shouldn't this Executor shut down when no longer needed? Also having more threads than cores may actually reduce the chance of hitting the desired concurrency effect because of the increased context switching among threads.
There was a problem hiding this comment.
Yes it should; first let's agree if this PR makes sense or not -- as we started in the ticket, if not, no reason to spend time polishing it ;-)
|
Closing, no need to guard against this -- it rides on guarantees made by Publisher in 1.3 |
Test implementation for #427 TCK should stress subscriber rule 2.5 in concurrent setting
Discussion so far in the ticket.