preWait not working since Ktor v2.3.0 by default

[mivanilov]

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:

• Start app with -Dio.ktor.server.engine.ShutdownHook=true (true by default)
• curl http://localhost/ping -v to get 200 response
• kill -s SIGTERM `lsof -t -i:80`
• curl http://localhost/ping -v again to get an error while it is expected to get 200 response for a period of preWait duration before engine.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.

[nomisRev]

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?

[mivanilov]

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.

[nomisRev]

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 👀

[tKe]

io.ktor.server.engine.ShutdownHook is only used on JVM - native ktor servers will currently always inject the SIGINT handler. With this in mind it may be more suitable to warn on startup in the event a shutdown hook is installed by ktor (i.e. is native, or the JVM property is true) rather than blowing up as that would always occur on native.