jooby-netty: possible leak in scheduled tasks under HTTP traffic

- only affect 4.x
- fix #3871

Author: jknack

modules/jooby-netty/src/main/java/io/jooby/internal/netty/NettyDateService.java

@@ -8,18 +8,25 @@
 import java.time.ZoneOffset;
 import java.time.ZonedDateTime;
 import java.time.format.DateTimeFormatter;
-import java.util.concurrent.ScheduledExecutorService;
-import java.util.concurrent.TimeUnit;
+import java.util.concurrent.*;
+
+import io.netty.util.concurrent.DefaultThreadFactory;
 
 public class NettyDateService implements Runnable {
   private static final int DATE_INTERVAL = 1000;
-  private CharSequence date;
+  private final ScheduledExecutorService scheduler;
+  // Make volatile for thread visibility across EventLoop threads
+  private volatile CharSequence date;
+  private final ScheduledFuture<?> future;
 
-  public NettyDateService(ScheduledExecutorService scheduler) {
+  public NettyDateService() {
+    this.scheduler =
+        Executors.newSingleThreadScheduledExecutor(new DefaultThreadFactory("http-date-keeper"));
     this.date =
         new NettyString(
             DateTimeFormatter.RFC_1123_DATE_TIME.format(ZonedDateTime.now(ZoneOffset.UTC)));
-    scheduler.scheduleAtFixedRate(this, DATE_INTERVAL, DATE_INTERVAL, TimeUnit.MILLISECONDS);
+    this.future =
+        scheduler.scheduleAtFixedRate(this, DATE_INTERVAL, DATE_INTERVAL, TimeUnit.MILLISECONDS);
   }
 
   public CharSequence date() {
@@ -32,4 +39,17 @@ public void run() {
         new NettyString(
             DateTimeFormatter.RFC_1123_DATE_TIME.format(ZonedDateTime.now(ZoneOffset.UTC)));
   }
+
+  // Allow the server to gracefully terminate the background task
+  public void stop() {
+    try {
+      if (this.future != null) {
+        this.future.cancel(false);
+      }
+    } catch (Exception ignored) {
+      if (scheduler != null) {
+        scheduler.shutdown();
+      }
+    }
+  }
 }

modules/jooby-netty/src/main/java/io/jooby/internal/netty/NettyPipeline.java

@@ -6,7 +6,6 @@
 package io.jooby.internal.netty;
 
 import java.util.List;
-import java.util.concurrent.ScheduledExecutorService;
 
 import io.jooby.Context;
 import io.netty.buffer.ByteBuf;
@@ -32,6 +31,7 @@ public class NettyPipeline extends ChannelInitializer<SocketChannel> {
   private final boolean http2;
   private final boolean expectContinue;
   private final Integer compressionLevel;
+  private final NettyDateService dateService;
 
   public NettyPipeline(
       SslContext sslContext,
@@ -43,7 +43,8 @@ public NettyPipeline(
       boolean defaultHeaders,
       boolean http2,
       boolean expectContinue,
-      Integer compressionLevel) {
+      Integer compressionLevel,
+      NettyDateService dateService) {
     this.sslContext = sslContext;
     this.decoderConfig = decoderConfig;
     this.contextSelector = contextSelector;
@@ -54,6 +55,7 @@ public NettyPipeline(
     this.http2 = http2;
     this.expectContinue = expectContinue;
     this.compressionLevel = compressionLevel;
+    this.dateService = dateService;
   }
 
   @Override
@@ -74,7 +76,7 @@ public void initChannel(SocketChannel ch) {
   private void setupHttp11(ChannelPipeline p) {
     p.addLast("codec", createServerCodec());
     addCommonHandlers(p);
-    p.addLast("handler", createHandler(p.channel().eventLoop()));
+    p.addLast("handler", createHandler());
   }
 
   private void setupHttp2(ChannelPipeline pipeline) {
@@ -100,7 +102,7 @@ private void setupHttp11Upgrade(ChannelPipeline pipeline) {
             (int) maxRequestSize));
 
     addCommonHandlers(pipeline);
-    pipeline.addLast("handler", createHandler(pipeline.channel().eventLoop()));
+    pipeline.addLast("handler", createHandler());
   }
 
   private ChannelInboundHandler setupHttp2Handshake(boolean secure) {
@@ -126,9 +128,9 @@ private Http2ServerUpgradeCodec createH2CUpgradeCodec() {
         new Http2MultiplexHandler(new Http2StreamInitializer(this)));
   }
 
-  private NettyHandler createHandler(ScheduledExecutorService executor) {
+  private NettyHandler createHandler() {
     return new NettyHandler(
-        new NettyDateService(executor),
+        dateService,
         contextSelector,
         maxRequestSize,
         maxFormFields,
@@ -193,7 +195,7 @@ private static class Http2StreamInitializer extends ChannelInitializer<Channel>
     @Override
     protected void initChannel(Channel ch) {
       ch.pipeline().addLast("http2", new Http2StreamFrameToHttpObjectCodec(true));
-      ch.pipeline().addLast("handler", pipeline.createHandler(ch.eventLoop()));
+      ch.pipeline().addLast("handler", pipeline.createHandler());
     }
   }
 }

modules/jooby-netty/src/main/java/io/jooby/netty/NettyServer.java

@@ -61,6 +61,7 @@ public class NettyServer extends Server.Base {
   private NettyOutputFactory outputFactory;
   private boolean singleEventLoopGroup =
       System.getProperty("io.netty.eventLoopGroup", "parent-child").equals("single");
+  private NettyDateService dateLoop;
 
   /**
    * Creates a server.
@@ -147,6 +148,7 @@ public Server start(@NonNull Jooby... application) {
             new NettyEventLoopGroupImpl(
                 transport, singleEventLoopGroup, options.getIoThreads(), worker);
       }
+      this.dateLoop = new NettyDateService();
 
       fireStart(List.of(application), eventLoop.worker());
 
@@ -232,7 +234,8 @@ private NettyPipeline newPipeline(ServerOptions options, SslContext sslContext,
         options.getDefaultHeaders(),
         http2,
         options.isExpectContinue() == Boolean.TRUE,
-        options.getCompressionLevel());
+        options.getCompressionLevel(),
+        dateLoop);
   }
 
   @Override
@@ -241,6 +244,10 @@ public synchronized Server stop() {
     // only for jooby build where close events may take longer.
     NettyWebSocket.all.clear();
 
+    if (this.dateLoop != null) {
+      this.dateLoop.stop();
+    }
+
     if (eventLoop != null) {
       eventLoop.shutdown();
     }