Comments (5)
Thanks @tsegismont ! Hopefully the last thing along these lines: I think you might be missing the following override in the new class ContextCoroutineDispatcher
.
override fun invokeOnTimeout(timeMillis: Long, block: Runnable, context: CoroutineContext): DisposableHandle {
return (delegate as Delay).invokeOnTimeout(timeMillis, block, context)
}
With the override the timeout functionality will run with io.vertx.kotlin.coroutines.VertxCoroutineExecutor#schedule(java.lang.Runnable, long, java.util.concurrent.TimeUnit)
While without the override, it will rely on DefaultDelay.invokeOnTimeout(timeMillis, block, context)
which is a different executor which gets involved.
For example, with the override and the code like this
try {
withTimeout(100) {
delay(1000)
}
} catch (e: TimeoutCancellationException) {
while without the override it's
![image](https://private-user-images.githubusercontent.com/72110/274451555-8e44cc9f-48ae-46a5-bd3c-52da820e8c7c.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MTUwNDg3NjYsIm5iZiI6MTcxNTA0ODQ2NiwicGF0aCI6Ii83MjExMC8yNzQ0NTE1NTUtOGU0NGNjOWYtNDhhZS00NmE1LWJkM2MtNTJkYTgyMGU4YzdjLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDA1MDclMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQwNTA3VDAyMjEwNlomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTIwZTU2NDUyMmQxNmRlMDIzOGVlOTk2OWQ2M2E2MzliYjIwMzM5ZjRlNzMwZjkxODFjZDQzNTYwNjNiMTIyNTcmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0JmFjdG9yX2lkPTAma2V5X2lkPTAmcmVwb19pZD0wIn0.f2cIF3XLBUWr7pBgeBXYSEh6VhHZoQDG7Dq0YHO83Jo)
The issue doesn't change correctness, to be sure, but users might expect the functionality to stay exclusively with the Vert.x threads
from vertx-lang-kotlin.
@bbyk Thanks for you thorough review
I've tried to reproduce but the test passes:
@Test
fun `test withTimeout execution dispatched on Vertx coroutine executor`(testContext: TestContext) {
val latch = testContext.async()
val context = (vertx as VertxInternal).getOrCreateContext()
val duplicatedContext = context.duplicate()
val promise = Promise.promise<Any>()
duplicatedContext.runOnContext {
GlobalScope.launch(vertx.dispatcher()) {
try {
withTimeout(100) {
promise.future().await()
}
} catch (e: TimeoutCancellationException) {
testContext.assertEquals(ContextInternal.current(), duplicatedContext)
latch.complete()
}
}
}
}
Can you share your reproducer?
from vertx-lang-kotlin.
I'd like to mention again that the missing override doesn't change correctness of
withTimeout
, it only leads to spawning a non-Vert.x thread for running timeouts which, I assume, is less desirable.
Got you. This will be fixed by #246
from vertx-lang-kotlin.
Fixed by #244
from vertx-lang-kotlin.
@tsegismont I am sharing it below for a different concurrency primitive. But first I'd like to mention again that the missing override doesn't change correctness of withTimeout
, it only leads to spawning a non-Vert.x thread for running timeouts which, I assume, is less desirable. You can put a breakpoint in kotlinx.coroutines.TimeoutCoroutine#run
to see what I see on the screenshot.
You can have a more contrived case, though, that actually fails in a test. E.g. the following test fails ( a kludge: when you paste the sketched code, please remove private
access modifier from VertxCoroutineExecutor
for the sake of the test run - I didn't feel like re-implementing it in-place ). If you un-comment the code in invokeOnTimeout
the test will start to pass.
@InternalCoroutinesApi
@Test
fun `test select-onTimeout execution dispatched on Vertx coroutine executor`(testContext: TestContext) {
val latch = testContext.async()
val context = (vertx as VertxInternal).getOrCreateContext()
val duplicatedContext = context.duplicate()
val invokeTimeoutOnDuplicatedContextDispatcher = object : CoroutineDispatcher(), Delay {
override fun dispatch(context: CoroutineContext, block: Runnable) {
testContext.fail()
}
override fun isDispatchNeeded(context: CoroutineContext): Boolean {
return false
}
override fun scheduleResumeAfterDelay(timeMillis: Long, continuation: CancellableContinuation<Unit>) {
testContext.fail()
}
override fun invokeOnTimeout(timeMillis: Long, block: Runnable, context: CoroutineContext): DisposableHandle {
return super.invokeOnTimeout(timeMillis, block, context)
// val delay = VertxCoroutineExecutor(duplicatedContext).asCoroutineDispatcher() as Delay
// return delay.invokeOnTimeout(timeMillis, block, context)
}
}
val promise = Promise.promise<Any>()
duplicatedContext.runOnContext {
GlobalScope.launch(vertx.dispatcher()) {
val neverEndingJob = launch {
promise.future().await()
}
withContext(invokeTimeoutOnDuplicatedContextDispatcher) {
// still on Vert.x
testContext.assertEquals(duplicatedContext, ContextInternal.current())
select<Unit> {
neverEndingJob.onJoin {
}
onTimeout(100) {
testContext.assertEquals(duplicatedContext, ContextInternal.current())
neverEndingJob.cancel()
latch.complete()
}
}
}
}
}
}
from vertx-lang-kotlin.
Related Issues (20)
- json builder processes binary data differently from JsonObject class HOT 3
- Update to Kotlin 1.5.31
- Provide a vertxFuture coroutine builder HOT 2
- ReaderStream<T>.toReceiveChannel return elements in random order HOT 6
- CVE-2021-45105
- CVE-2021-45105
- CVE-2021-45105
- Starting a coroutine on a vert.x worker thread runs it on the same thread but should hand it over to the event loop thread HOT 3
- Upgrade to Kotlin 1.7.21
- Upgrade to Kotlin Coroutines 1.6.4
- Support for Mutiny for CoroutineVerticle for awaitSuspending Support HOT 1
- Add Vert.x core dependency back HOT 1
- Upgrade to 4.4.0 requires stdlib HOT 5
- StackOverflowError caused by two yield calls HOT 4
- Coroutine dispatcher doesn't execute commands on the duplicated context HOT 3
- ChannelWriteStream has a bug closing underlying stream HOT 3
- Add Future.coAwait and deprecate Future.await HOT 1
- Upgrade to Kotlin 1.9
- Kotlin fixes HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from vertx-lang-kotlin.