Moving Play's AkkaHttpServer#stop to CoordinatedShutdown

AkkaHttpServer.patch

diff --git a/framework/src/play-akka-http-server/src/main/scala/play/core/server/AkkaHttpServer.scala b/framework/src/play-akka-http-server/src/main/scala/play/core/server/AkkaHttpServer.scala
index d4f495c35..b5aca371b 100644
--- a/framework/src/play-akka-http-server/src/main/scala/play/core/server/AkkaHttpServer.scala
+++ b/framework/src/play-akka-http-server/src/main/scala/play/core/server/AkkaHttpServer.scala
@@ -6,12 +6,13 @@ package play.core.server
 
 import java.net.InetSocketAddress
 import java.security.{ Provider, SecureRandom }
-import javax.net.ssl._
 
-import akka.actor.ActorSystem
+import akka.Done
+import javax.net.ssl._
+import akka.actor.{ ActorSystem, CoordinatedShutdown }
 import akka.http.play.WebSocketHandler
 import akka.http.scaladsl.model.{ headers, _ }
-import akka.http.scaladsl.model.headers.Expect
+import akka.http.scaladsl.model.headers.{ Expect, `Remote-Address` }
 import akka.http.scaladsl.model.ws.UpgradeToWebSocket
 import akka.http.scaladsl.settings.{ ParserSettings, ServerSettings }
 import akka.http.scaladsl.util.FastFuture._
@@ -62,6 +63,8 @@ class AkkaHttpServer(
 
   private val http2Enabled: Boolean = akkaServerConfig.getOptional[Boolean]("http2.enabled") getOrElse false
 
+  registerShutdownTasks()
+
   /**
    * The underlying Config object used to initialize Akka HTTP. We patch in a setting to enable
    * or disable HTTP/2.
@@ -341,30 +344,74 @@ class AkkaHttpServer(
   }
 
   override def stop(): Unit = {
+    // CoordinatedShutdown may be invoked many times over the same actorSystem but
+    // only the first invocation runs the tasks (later invocations are noop).
+    CoordinatedShutdown(actorSystem).run(CoordinatedShutdown.UnknownReason)
+  }
 
-    mode match {
-      case Mode.Test =>
-      case _ => logger.info("Stopping server...")
-    }
+  // Using CoordinatedShutdown means that instead of invoking code imperatively in `stop`
+  // we have to register it as early as possible as CoordinatedShutdown tasks and
+  // then `stop` tries to runs CoordinatedShutdown.
+  private def registerShutdownTasks(): Unit = {
+    // Register all shutdown tasks
+    CoordinatedShutdown(actorSystem)
+      .addTask(CoordinatedShutdown.PhaseBeforeServiceUnbind, "trace-server-stop-request") {
+        () =>
+          mode match {
+            case Mode.Test =>
+            case _ => logger.info("Stopping server...")
+          }
+          Future.successful(Done)
+      }
 
-    // First, stop listening
-    def unbind(binding: Http.ServerBinding) = Await.result(binding.unbind(), Duration.Inf)
-    httpServerBinding.foreach(unbind)
-    httpsServerBinding.foreach(unbind)
-    applicationProvider.current.foreach(Play.stop)
+    // Stop listening.
+    // TODO: this can be improved so unbind is deferred until `service-stop`. Doing that
+    // would allow in flight requests to complete and we should respond 503 in the meantime.
+    CoordinatedShutdown(actorSystem)
+      .addTask(CoordinatedShutdown.PhaseServiceUnbind, "akka-http-server-unbind") {
+        () =>
+          def unbind(binding: Http.ServerBinding) = Await.result(binding.unbind(), Duration.Inf)
+
+          httpServerBinding.foreach(unbind)
+          httpsServerBinding.foreach(unbind)
+          Future.successful(Done)
+      }
 
-    try {
-      super.stop()
-    } catch {
-      case NonFatal(e) => logger.error("Error while stopping logger", e)
-    }
+    // Registering a task that invokes `Play.stop` is necessary for the scenarios where
+    // the Application and the Server use separate ActorSystems (e.g. DevMode). Because
+    // it's safe to invoke CoordinatedShutdown many times over the same actorSystem and
+    // only the first invocation runs the tasks (later invocations are noop).
+    // Note that a Server MUST stop the Application it runs but an Application MAY stop
+    // the Server it runs on.
+    CoordinatedShutdown(actorSystem)
+      .addTask(CoordinatedShutdown.PhaseServiceStop, "shutdown-application-and-logger") {
+        () =>
+          // Invoking Play.stop(app) here doesn't mean all tasks registered in the
+          // Application will actually be run on this phase of CoordinatedShutdown
+          val stoppedApp = applicationProvider.get.map(Play.stop)
+          val stopLogger: Unit => Done = _ => {
+            try {
+              super.stop()
+            } catch {
+              case NonFatal(e) => logger.error("Error while stopping logger", e)
+            }
+            Done
+          }
+          Future.fromTry(stoppedApp).map(stopLogger)
+      }
 
     // Call provided hook
     // Do this last because the hooks were created before the server,
     // so the server might need them to run until the last moment.
-    Await.result(stopHook(), Duration.Inf)
+    CoordinatedShutdown(actorSystem)
+      .addTask(CoordinatedShutdown.PhaseBeforeActorSystemTerminate, "user-provided-server-stop-hook") {
+        () => stopHook().map(_ => Done)
+      }
   }
 
+
+
+
   override lazy val mainAddress: InetSocketAddress = {
     httpServerBinding.orElse(httpsServerBinding).map(_.localAddress).get
   }