typesafehub / akka-contrib-extra

ConductR Akka contributions
Other
9 stars 16 forks source link

NonBlockingProcess not killing child processes on postStop #72

Open MasseGuillaume opened 7 years ago

MasseGuillaume commented 7 years ago

I don't have a test case to confirm this yet but this is what I observe:

to reproduce: I run an infinite computation I throw an exception to trigger the actor restart (default supervision strategy) the postStop is called and kill the process

observed: the process is still running despite the process.destroy(true)

my hypothesis is that we need to wait for the process to be fully stopped (by blocking in the postStop with a latch for example).

I'm still investigating, I will let you know if I find anything.

huntc commented 7 years ago

We should document this, but you need to explicitly tell your process actor to stop. Right @longshorej?

huntc commented 7 years ago

I might have that wrong though... killing the actor sys should be sufficient.

longshorej commented 7 years ago

Yeah, when the actor shuts down that should kill the process via postStop invoking process.destroy(true). Can you provide a small reproducer project @MasseGuillaume?

MasseGuillaume commented 7 years ago

@longshorej is correct, process.destroy(true) is called. However, we don't wait for the process to exit.

MasseGuillaume commented 7 years ago

The process in question is sbt. The sbt bash script spawns a java process. destroy won't destroy child processes. Before I was using pkill to do this.

MasseGuillaume commented 7 years ago

https://github.com/brettwooldridge/NuProcess/issues/78

MasseGuillaume commented 7 years ago

NB. since the child actors are stoped first the process will receive a kill first. If you do pkill -P on the defunct process it will not kill its children.

huntc commented 7 years ago

Subsequently forked processes will have to be killed by the process you launch from within this actor. Signals don’t get propagated. Perhaps you need to launch a script that will trap and pkill-P for its sub processes.

I think that this behaviour is conventional.

MasseGuillaume commented 7 years ago

This is what I end up doing:

package com.olegych.scastie.util

import akka.contrib.process.NonBlockingProcess
import java.nio.file.{Path, Paths}
import scala.collection.immutable

import akka.actor.Props

object NonBlockingProcessPkill {
  def props(command: immutable.Seq[String],
            workingDir: Path = Paths.get(System.getProperty("user.dir")),
            environment: Map[String, String] = Map.empty) =
    Props(new NonBlockingProcessPkill(command, workingDir, environment))
}

class NonBlockingProcessPkill(command: immutable.Seq[String],
                              workingDir: Path,
                              environment: Map[String, String])
    extends NonBlockingProcess(command, workingDir, environment) {
  override def postStop(): Unit = {
    // https://github.com/typesafehub/akka-contrib-extra/issues/72

    import sys.process._
    s"pkill -KILL -P ${process.getPID}".!

    super.postStop()
  }
}
huntc commented 7 years ago

Hmm. You probably should still consider some supervisor script. Here’s something else to think about: https://github.com/typesafehub/akka-contrib-extra/issues/70