Simplify UnaryBlockingCall #225
Conversation
jhump
force-pushed
the
jh/fix-unary-call
branch
2 times, most recently
from
89894c1 to
6e86012
Compare
jhump
force-pushed
the
jh/fix-unary-call
branch
2 times, most recently
from
9b849c8 to
4d5286b
Compare
- It's now just an interface, and the implementation is in the impl package (just like the stream interfaces). - The implementation is simpler and thread-safe. - No longer supports calling it more than once. This was incorrectly implemented before since it became impossible to cancel earlier invocations (and was not thread-safe). Much simpler to just not allow this than to fix it.
jhump
force-pushed
the
jh/fix-unary-call
branch
from
4d5286b to
a885a81
Compare
… basic unit tests
jhump
force-pushed
the
jh/fix-unary-call
branch
from
a885a81 to
4af7f5d
Compare
| ) | ||
| return@submit | ||
| } | ||
| callback.invoke( |
There was a problem hiding this comment.
Could this test be simplified to just call success on the callback without a delay? (So remove try/catch block above). Since nothing calls cancel, that shouldn't ever be invoked.
| val call = UnaryCall<Any> { callback -> | ||
| val future = executor.submit { | ||
| try { | ||
| Thread.sleep(1_000L) |
There was a problem hiding this comment.
I wonder if it would be possible to coordinate these threads without relying on sleep. Maybe a countdown latch which is counted down here (before it just calls .wait() indefinitely). Then in the execution thread below instead of a sleep on line 108 we wait on the countdown latch to signal that this callable has started and will be interrupted when we call cancel.
There was a problem hiding this comment.
Done. I also added another test for the case where call.cancel is invoked before call.execute.
I found the implementation for this (with mutator methods and a kind of funky interaction between execute and cancel) a little confusing.