HADOOP-19318. Remove usage of sun.misc.Signal

apache · Nov 5, 2024 · 197dbf1 · 197dbf1
1 parent 9ae01bd
commit 197dbf1
Show file tree

Hide file tree

Showing 6 changed files with 54 additions and 44 deletions.
diff --git a/hadoop-common-project/hadoop-common/pom.xml b/hadoop-common-project/hadoop-common/pom.xml
@@ -416,6 +416,12 @@
       <artifactId>lz4-java</artifactId>
       <scope>provided</scope>
     </dependency>
+    <dependency>
+      <groupId>com.github.jnr</groupId>
+      <artifactId>jnr-posix</artifactId>
+      <version>3.1.20</version>
+    </dependency>
+
   </dependencies>
 
   <build>

diff --git a/...ct/hadoop-common/src/main/java/org/apache/hadoop/service/launcher/InterruptEscalator.java b/...ct/hadoop-common/src/main/java/org/apache/hadoop/service/launcher/InterruptEscalator.java
@@ -23,6 +23,7 @@
 import java.util.List;
 import java.util.concurrent.atomic.AtomicBoolean;
 
+import jnr.constants.platform.Signal;
 import org.apache.hadoop.util.Preconditions;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -140,7 +141,7 @@ public void interrupted(IrqHandler.InterruptData interruptData) {
    * @throws IllegalArgumentException if the registration failed
    */
   public synchronized void register(String signalName) {
-    IrqHandler handler = new IrqHandler(signalName, this);
+    IrqHandler handler = new IrqHandler(Signal.valueOf(signalName), this);
     handler.bind();
     interruptHandlers.add(handler);
   }

diff --git a/...on-project/hadoop-common/src/main/java/org/apache/hadoop/service/launcher/IrqHandler.java b/...on-project/hadoop-common/src/main/java/org/apache/hadoop/service/launcher/IrqHandler.java
@@ -20,11 +20,13 @@
 
 import java.util.concurrent.atomic.AtomicInteger;
 
+import jnr.constants.platform.Signal;
+import jnr.posix.POSIX;
+import jnr.posix.POSIXFactory;
+import jnr.posix.SignalHandler;
 import org.apache.hadoop.util.Preconditions;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import sun.misc.Signal;
-import sun.misc.SignalHandler;
 
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.classification.InterfaceStability;
@@ -45,22 +47,18 @@ public final class IrqHandler implements SignalHandler {
   /**
    * Definition of the Control-C handler name: {@value}.
    */
-  public static final String CONTROL_C = "INT";
+  public static final Signal CONTROL_C = Signal.SIGINT;
 
   /**
    * Definition of default <code>kill</code> signal: {@value}.
    */
-  public static final String SIGTERM = "TERM";
-
-  /**
-   * Signal name.
-   */
-  private final String name;
+  public static final Signal SIGTERM = Signal.SIGTERM;
 
   /**
    * Handler to relay to.
    */
   private final Interrupted handler;
+  private static final POSIX posix = POSIXFactory.getNativePOSIX();
 
   /** Count of how many times a signal has been raised. */
   private final AtomicInteger signalCount = new AtomicInteger(0);
@@ -72,28 +70,26 @@ public final class IrqHandler implements SignalHandler {
 
   /**
    * Create an IRQ handler bound to the specific interrupt.
-   * @param name signal name
+   * @param signal signal
    * @param handler handler
    */
-  public IrqHandler(String name, Interrupted handler) {
-    Preconditions.checkArgument(name != null, "Null \"name\"");
+  public IrqHandler(Signal signal, Interrupted handler) {
+    Preconditions.checkArgument(signal != null, "Null \"signal\"");
     Preconditions.checkArgument(handler != null, "Null \"handler\"");
     this.handler = handler;
-    this.name = name;
+    this.signal = signal;
   }
 
   /**
    * Bind to the interrupt handler.
    * @throws IllegalArgumentException if the exception could not be set
    */
   public void bind() {
-    Preconditions.checkState(signal == null, "Handler already bound");
     try {
-      signal = new Signal(name);
-      Signal.handle(signal, this);
+      posix.signal(signal, this);
     } catch (IllegalArgumentException e) {
       throw new IllegalArgumentException(
-          "Could not set handler for signal \"" + name + "\"."
+          "Could not set handler for signal \"" + signal + "\"."
           + "This can happen if the JVM has the -Xrs set.",
           e);
     }
@@ -103,29 +99,29 @@ public void bind() {
    * @return the signal name.
    */
   public String getName() {
-    return name;
+    return signal.name();
   }
 
   /**
    * Raise the signal.
    */
   public void raise() {
-    Signal.raise(signal);
+    posix.kill(posix.getpid(), signal.intValue());
   }
 
   @Override
   public String toString() {
-    return "IrqHandler for signal " + name;
+    return "IrqHandler for signal " + signal.name();
   }
 
   /**
    * Handler for the JVM API for signal handling.
-   * @param s signal raised
+   * @param signalNumber signal raised
    */
   @Override
-  public void handle(Signal s) {
+  public void handle(int signalNumber) {
     signalCount.incrementAndGet();
-    InterruptData data = new InterruptData(s.getName(), s.getNumber());
+    InterruptData data = new InterruptData(signal.name(), signalNumber);
     LOG.info("Interrupted: {}", data);
     handler.interrupted(data);
   }

diff --git a/...oject/hadoop-common/src/main/java/org/apache/hadoop/service/launcher/ServiceLauncher.java b/...oject/hadoop-common/src/main/java/org/apache/hadoop/service/launcher/ServiceLauncher.java
@@ -761,8 +761,8 @@ protected void registerFailureHandling() {
     try {
       interruptEscalator = new InterruptEscalator(this,
           SHUTDOWN_TIME_ON_INTERRUPT);
-      interruptEscalator.register(IrqHandler.CONTROL_C);
-      interruptEscalator.register(IrqHandler.SIGTERM);
+      interruptEscalator.register(IrqHandler.CONTROL_C.name());
+      interruptEscalator.register(IrqHandler.SIGTERM.name());
     } catch (IllegalArgumentException e) {
       // downgrade interrupt registration to warnings
       LOG.warn("{}", e, e);

diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/SignalLogger.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/SignalLogger.java
@@ -18,13 +18,17 @@
 
 package org.apache.hadoop.util;
 
+import jnr.constants.platform.Signal;
+import jnr.posix.POSIX;
+import jnr.posix.POSIXFactory;
+import jnr.posix.SignalHandler;
 import org.slf4j.Logger;
-import sun.misc.Signal;
-import sun.misc.SignalHandler;
-
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.classification.InterfaceStability;
 
+import java.util.EnumSet;
+import java.util.Set;
+
 /**
  * This class logs a message whenever we're about to exit on a UNIX signal.
  * This is helpful for determining the root cause of a process' exit.
@@ -35,6 +39,9 @@
 @InterfaceStability.Unstable
 public enum SignalLogger {
   INSTANCE;
+  private static final Set<Signal> SIGNALS = EnumSet.of(Signal.SIGHUP, Signal.SIGINT, Signal.SIGTERM);
+  private static final POSIX POSIX_IMPL = POSIXFactory.getJavaPOSIX();
+  private static final SignalHandler DEFAULT_HANDLER = System::exit;
 
   private boolean registered = false;
 
@@ -45,9 +52,10 @@ private static class Handler implements SignalHandler {
     final private Logger log;
     final private SignalHandler prevHandler;
 
-    Handler(String name, Logger log) {
+    Handler(Signal signal, Logger log) {
       this.log = log;
-      prevHandler = Signal.handle(new Signal(name), this);
+      SignalHandler handler = POSIX_IMPL.signal(signal, this);
+      prevHandler = handler != null ? handler : DEFAULT_HANDLER;
     }
 
     /**
@@ -56,9 +64,8 @@ private static class Handler implements SignalHandler {
      * @param signal    The incoming signal
      */
     @Override
-    public void handle(Signal signal) {
-      log.error("RECEIVED SIGNAL " + signal.getNumber() +
-          ": SIG" + signal.getName());
+    public void handle(int signal) {
+      log.error("RECEIVED SIGNAL {}: SIG{}", signal, Signal.valueOf(signal));
       prevHandler.handle(signal);
     }
   }
@@ -75,16 +82,15 @@ public void register(final Logger log) {
     registered = true;
     StringBuilder bld = new StringBuilder();
     bld.append("registered UNIX signal handlers for [");
-    final String SIGNALS[] = { "TERM", "HUP", "INT" };
     String separator = "";
-    for (String signalName : SIGNALS) {
+    for (Signal signal : SIGNALS) {
       try {
-        new Handler(signalName, log);
+        new Handler(signal, log);
         bld.append(separator)
-            .append(signalName);
+            .append(signal.name());
         separator = ", ";
       } catch (Exception e) {
-        log.debug("Error: ", e);
+        log.info("Error installing UNIX signal handler for {}", signal, e);
       }
     }
     bld.append("]");

diff --git a/...common/src/test/java/org/apache/hadoop/service/launcher/TestServiceInterruptHandling.java b/...common/src/test/java/org/apache/hadoop/service/launcher/TestServiceInterruptHandling.java
@@ -18,6 +18,7 @@
 
 package org.apache.hadoop.service.launcher;
 
+import jnr.constants.platform.Signal;
 import org.apache.hadoop.service.BreakableService;
 import org.apache.hadoop.service.launcher.testservices.FailureTestService;
 import org.apache.hadoop.test.GenericTestUtils;
@@ -38,8 +39,8 @@ public class TestServiceInterruptHandling
   @Test
   public void testRegisterAndRaise() throws Throwable {
     InterruptCatcher catcher = new InterruptCatcher();
-    String name = "USR2";
-    IrqHandler irqHandler = new IrqHandler(name, catcher);
+    String name = "SIGINT";
+    IrqHandler irqHandler = new IrqHandler(Signal.valueOf(name), catcher);
     irqHandler.bind();
     assertEquals(0, irqHandler.getSignalCount());
     irqHandler.raise();
@@ -61,7 +62,7 @@ public void testInterruptEscalationShutdown() throws Throwable {
 
     // call the interrupt operation directly
     try {
-      escalator.interrupted(new IrqHandler.InterruptData("INT", 3));
+      escalator.interrupted(new IrqHandler.InterruptData("SIGINT", 3));
       fail("Expected an exception to be raised in " + escalator);
     } catch (ExitUtil.ExitException e) {
       assertExceptionDetails(EXIT_INTERRUPTED, "", e);
@@ -75,7 +76,7 @@ public void testInterruptEscalationShutdown() throws Throwable {
 
     // now interrupt it a second time and expect it to escalate to a halt
     try {
-      escalator.interrupted(new IrqHandler.InterruptData("INT", 3));
+      escalator.interrupted(new IrqHandler.InterruptData("SIGINT", 3));
       fail("Expected an exception to be raised in " + escalator);
     } catch (ExitUtil.HaltException e) {
       assertExceptionDetails(EXIT_INTERRUPTED, "", e);
@@ -93,7 +94,7 @@ public void testBlockingShutdownTimeouts() throws Throwable {
     InterruptEscalator escalator = new InterruptEscalator(launcher, 500);
     // call the interrupt operation directly
     try {
-      escalator.interrupted(new IrqHandler.InterruptData("INT", 3));
+      escalator.interrupted(new IrqHandler.InterruptData("SIGINT", 3));
       fail("Expected an exception to be raised from " + escalator);
     } catch (ExitUtil.ExitException e) {
       assertExceptionDetails(EXIT_INTERRUPTED, "", e);