Open mivanilov opened 6 months ago
Hey @mivanilov,
Thank you for the report!! We could potentially also manually put Dio.ktor.server.engine.ShutdownHook
to false, and print a warning if the user left it to true
. 🤔 Or some other kind of logging.
What woud you expect to happen? It's not clear to me from your comment, or would you prefer it to blow up?
Hey @nomisRev looking at the Ktor
engines and SuspendApp
code as it is now, it makes sense to me set Dio.ktor.server.engine.ShutdownHook
to false
as ultimately SuspendApp
delays engine.stop
call - which is the same function that is registered with the Ktor shutdown hook e.g. for Netty https://github.com/ktorio/ktor/blob/main/ktor-server/ktor-server-netty/jvm/src/io/ktor/server/netty/NettyApplicationEngine.kt#L214
Also I would probably prefer it blow up if Dio.ktor.server.engine.ShutdownHook
is set to true
as in such case SuspendApp
just won't work as there will be no expected delay before engine.stop
call.
Hey @mivanilov,
Thank you for letting me know your thoughts, I 100% agree. I am going to implement this in the next month or so, while we work on releasing Arrow 2.0 alongside the K2 release that'll come any week now. KotlinConf 👀
Since Ktor v2.3.0 shutdown hook was added to Ktor engines e.g. CIO and Netty, making Ktor server to stop before waiting a
preWait
duration configured by SuspendApp.Adding an example project to reproduce this issue issue-demo.zip
Steps to reproduce it:
-Dio.ktor.server.engine.ShutdownHook=true
(true by default)curl http://localhost/ping -v
to get 200 responsecurl http://localhost/ping -v
again to get an error while it is expected to get 200 response for a period ofpreWait
duration beforeengine.stop
is called.Setting
-Dio.ktor.server.engine.ShutdownHook=false
fixes this issue.To save developers time troubleshooting this issue, probably SuspendApp readme/docs should mention this Ktor engine configuration bit making sure SuspendApp works as expected?
Also might be worth adding an integration test to cover this behaviour.