apache / maven-mvnd

Apache Maven Daemon
https://maven.apache.org/
Apache License 2.0
2.91k stars 212 forks source link

Cancel job on client TERM signal #1112

Open mbien opened 2 months ago

mbien commented 2 months ago

Hello!

the client is cancelling jobs on ctrl+c which makes sense. However, it seems like it won't do anything on TERM signal.

This makes it difficult for tools which treat mvn and mvnd the same to stop jobs, e.g (https://github.com/apache/netbeans/issues/7676).

This would be trivial to solve with a shutdown hook, but while playing with this, some problems occurred:

here is a prototype which:

  • adds a JVM shutdown hook which sends the cancel msg on irregular shutdown (SIGTERM)
  • this requires to build the native image with --install-exit-handlers, since without it, GraalVM would not trigger shutdown hooks on irregular shutdowns

The problem however is that the build is also setting -H:-ParseRuntimeOptions for the purpose of delegating -D params instead of parsing them as args. This seems to mess with the signal chaining and prevents the shutdown hook from running again.

This almost looks like a graalvm bug to me - but I am not sure. Maybe someone here has more experience with this scenario. (I tried graalvm 22.0.2 and also some older releases).

If the mvnd team thinks this is the right direction I would try to investigate a bit further if this is a graalvm bug or not.

(There are also many ways how to solve this from the side of NetBeans, it could send "ctrl+c" first, however there is probably no way to know how long to wait before sending TERM if nothing happens. Or it could talk to the daemon itself. I think it would be more elegant if both mvn and mvnd would be treated the same though in terms of shutdown.)

cstamas commented 2 months ago

@gnodet @ppalaga please chime in!

gnodet commented 2 months ago

@mbien your proposal sounds legit to me. I must admit my GraalVM experience is mostly limited to mvnd, so I won't be able to help much on that side.

mbien commented 2 months ago

thanks! I could reproduce it outside of mvnd and filed a bug upstream https://github.com/oracle/graal/issues/9568