Repository: tipsy/javalin (kotlin)

Issue:
Clean up timed out futures and/or expose Consumer<AsyncContext>
Issue raised by @dancohensmith on Gitter. If a request with a future result times out, the future could/should be rejected/cancelled. This could either be done automatically in `JavalinServlet`, or by exposing `Consumer<AsyncContext>` to the user. The user could then add listeners to the `AsyncContext`.

Pull request description:
Closes #585 .

Patch:
diff --git a/javalin/src/main/java/io/javalin/http/JavalinServlet.kt b/javalin/src/main/java/io/javalin/http/JavalinServlet.kt
index 3e5290493..33b2ab003 100644
--- a/javalin/src/main/java/io/javalin/http/JavalinServlet.kt
+++ b/javalin/src/main/java/io/javalin/http/JavalinServlet.kt
@@ -63,7 +63,13 @@ class JavalinServlet(val config: JavalinConfig) : HttpServlet() {
                 throw NotFoundResponse()
             }
 
+            var responseStarted = false
+
             fun finishUpResponse(response: ServletResponse) {
+                // Avoiding the possibility of writing to the response stream on both asynchronous requests and errors
+                if (responseStarted) return
+
+                responseStarted = true
                 tryWithExceptionMapper { // run error mappers (can mutate ctx)
                     errorMapper.handle(ctx.status(), ctx)
                 }
@@ -72,7 +78,9 @@ class JavalinServlet(val config: JavalinConfig) : HttpServlet() {
                         entry.handler.handle(ContextUtil.update(ctx, entry, requestUri))
                     }
                 }
-                JavalinResponseWrapper(response as HttpServletResponse, rwc).write(ctx.resultStream()) // write the response
+
+                // write the response
+                JavalinResponseWrapper(response as HttpServletResponse,rwc).write(ctx.resultStream())
                 config.inner.requestLogger?.handle(ctx, LogUtil.executionTimeMs(ctx)) // log stuff
             }
 
@@ -85,8 +93,10 @@ class JavalinServlet(val config: JavalinConfig) : HttpServlet() {
             // user called Context#future, we call startAsync and setup callbacks
             val asyncContext = req.startAsync().apply { timeout = config.asyncRequestTimeout }
             asyncContext.addTimeoutListener {
+                val future = ctx.resultFuture()!! // we need the future before it is overwritten by `result` below
                 ctx.status(500).result("Request timed out")
                 finishUpResponse(asyncContext.response).also { asyncContext.complete() }
+                future.cancel(true)
             }
             ctx.resultFuture()!!.exceptionally { throwable ->
                 exceptionMapper.handleFutureException(ctx, throwable)


Test patch:
diff --git a/javalin/src/test/java/io/javalin/TestFuture.kt b/javalin/src/test/java/io/javalin/TestFuture.kt
index 1d30a721a..c0aa8a4ab 100644
--- a/javalin/src/test/java/io/javalin/TestFuture.kt
+++ b/javalin/src/test/java/io/javalin/TestFuture.kt
@@ -126,6 +126,19 @@ class TestFuture {
         assertThat(http.get("/").body).isEqualTo("My own simple error message")
     }
 
+    @Test
+    fun `timed out futures are canceled`() = TestUtil.test(impatientServer) { app, http ->
+        val future = CompletableFuture<String>()
+        app.get("/") {
+            Executors.newSingleThreadScheduledExecutor().schedule({
+                future.complete("Test")
+            }, 50, TimeUnit.MILLISECONDS)
+            it.future(future)
+        }
+        assertThat(http.get("/").body).isEqualTo("Request timed out")
+        assertThat(future.isCancelled).isTrue()
+    }
+
     private fun getFuture(result: String?, delay: Long = 10): CompletableFuture<String> {
         val future = CompletableFuture<String>()
         Executors.newSingleThreadScheduledExecutor().schedule({