Support shutdown hooks with signals

[ekrich]

Now you can use Ctrl-C or kill -s SIGTERM <pid> and shutdown hooks will be called and the process will exit gracefully.

Here is the sandbox I used - not sure how to test.

object Test {
  def main(args: Array[String]): Unit = {
    println("Shutdown test - use Ctrl-C or send SIGTERM")
    // Runtime
    //   .getRuntime()
    //   .addShutdownHook(new Thread() {
    //     override def run(): Unit = messageThread()
    //   })
    // or this
    sys.addShutdownHook(messageThread())
    while (true) {
      try Thread.sleep(1000)
      catch {
        case e: InterruptedException => {
          println("Interrupted")
        }
      }
    }
  }
  def messageThread() = (0 to 5).foreach { i =>
    println("Shutting down")
    Thread.sleep(500)
  }
}

[WojciechMazur]

Overall LGTM, only 2 things I don't think are correct

[ekrich]

This seems to work now. Avoid race without calling shutdown hooks twice and call exit after handling shutdown hooks. Not sure if this is 100% correct but it seems ok.

[ekrich]

I'll need to fix this but not sure what value to put. test-tools for ++2.13.13.

[error] Test scala.scalanative.linker.MinimalRequiredSymbolsTest.multithreading failed: java.lang.AssertionError: 
[error] Found more symbols then expected, config={debugMetadata=false, multithreading=true, targetTriple=x86_64-unknown-unknown}:
[error] Expected at most: {types=1050, members=6600, total=7650}
[error] Found:            {types=1028, members=6640, total=7668}
[error] Diff:             {types=-22, members=40, total=18}

[WojciechMazur]

For the tools tests it's fine to just update the values to the number of found symbols/classes + alignment to % 50 so small changes won't affect it much. Maybe we should remove this test.

Also, this use cases seems to be good candidate for scripted test. We probably should be able to create a custom sbt task to start compiled program (hopefully using multitple threads running in the lop) and create a task to run Process.destroy - I guess it should be sending the SIGINT / SIGTERM signal

[WojciechMazur]

I think it won't work when we'll have multiple threads running. Starting runHooks would also start scala.scanative.runtime.JoinNonDaemonThreads which would wait for all thread to finish. We should at least remove this shutdown hook.
Also signal handler might be run by any of the threads. Which might affect its execution.

[catap]

Can it help with #2483 ?

[catap]

Suggested change

	lazy val setupSignalHandler = {
	signal.signal(signal.SIGINT, handleSignal(_))
	signal.signal(signal.SIGTERM, handleSignal(_))
	}
	lazy val setupSignalHandler = {
	signal.signal(signal.SIGHUP, handleSignal(_))
	signal.signal(signal.SIGINT, handleSignal(_))
	signal.signal(signal.SIGQUIT, handleSignal(_))
	signal.signal(signal.SIGTERM, handleSignal(_))
	}

let's add a few more expected reason to quit

[ekrich]

As far as I understand "The JVM runs shutdown hooks only in case of normal terminations." - what this seems to mean is Ctrl-C SIGINT and SIGTERM. This is also in the current Test comments.

[catap]

The JVM catches signals to implement shutdown hooks for unexpected termination. The JVM uses SIGHUP, SIGINT, and SIGTERM to initiate the running of shutdown hooks.
Applications embedding the JVM frequently need to trap signals such as SIGINT or SIGTERM, which can lead to interference with the JVM signal handlers. The -Xrs option is available to address this issue. When -Xrs is used, the signal masks for SIGINT, SIGTERM, SIGHUP, and SIGQUIT aren't changed by the JVM, and signal handlers for these signals aren't installed.

https://docs.oracle.com/en/java/javase/21/docs/specs/man/java.html

[ekrich]

Ok, I'll add SIGHUP.

[ekrich]

On second thought, we need to try and get this to work properly this way first as we are using clib and SIGHUP is not a member. I second pass could use posixlib and something special for Windows to make it JVM compliant.

[catap]

Keep in mind that SIGHUP is a signal which is send when SSH connection is hand up due network issue.

Without it, shutdown hook at CLI utilite won't work on this case and it's show stoper for real life use case I guess.

[ekrich]

No problem but we have nothing now and to make it cross platform is more complex so it might be better to have something now.

[WojciechMazur]

LGTM for the single threaded execution, I have prepared a variant handling the multithreaded case including handling on the GC side. I'll add the follow up PR soon

[ekrich]

Thanks, I was looking into the multi-threaded version and it was quite a bit more involved.