Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HADOOP-19329. Remove usage of sun.misc.Signal #7145

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions LICENSE-binary
Original file line number Diff line number Diff line change
Expand Up @@ -521,6 +521,11 @@ junit:junit:4.13.2
org.jacoco:org.jacoco.agent:0.8.5


Eclipse Public License 2.0
--------------------------

com.github.jnr:jnr-posix:3.1.19
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it also pulls transitive jars.

Honestly, it's a bit heavy to pull bunches of jars to just work around the access of sun.misc.Signal API. Could it be addressed by using reflection?



HSQL License
------------
Expand Down
1 change: 1 addition & 0 deletions hadoop-client-modules/hadoop-client-minicluster/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -709,6 +709,7 @@
<exclude>org.bouncycastle:*</exclude>
<!-- Leave snappy that includes native methods which cannot be relocated. -->
<exclude>org.xerial.snappy:*</exclude>
<exclude>com.github.jnr:*</exclude>
</excludes>
</artifactSet>
<filters>
Expand Down
1 change: 1 addition & 0 deletions hadoop-client-modules/hadoop-client-runtime/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@
<exclude>org.xerial.snappy:*</exclude>
<!-- leave out kotlin classes -->
<exclude>org.jetbrains.kotlin:*</exclude>
<exclude>com.github.jnr:*</exclude>
</excludes>
</artifactSet>
<filters>
Expand Down
5 changes: 5 additions & 0 deletions hadoop-common-project/hadoop-common/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,11 @@
<artifactId>lz4-java</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>com.github.jnr</groupId>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the license is tri EPL/GPL/LGPL license. Should be fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your review, and i have added com.github.jnr to GPLv2 module in LICENSE-binary file.

<artifactId>jnr-posix</artifactId>
</dependency>

</dependencies>

<build>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 String CONTROL_C = "SIGINT";

/**
* Definition of default <code>kill</code> signal: {@value}.
*/
public static final String SIGTERM = "TERM";

/**
* Signal name.
*/
private final String name;
public static final String SIGTERM = "SIGTERM";

/**
* Handler to relay to.
*/
private final Interrupted handler;
private static final POSIX POSIX_IMPL = POSIXFactory.getNativePOSIX();

/** Count of how many times a signal has been raised. */
private final AtomicInteger signalCount = new AtomicInteger(0);
Expand All @@ -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_IMPL.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);
}
Expand All @@ -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_IMPL.kill(POSIX_IMPL.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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -35,6 +39,10 @@
@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;

Expand All @@ -45,9 +53,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;
}

/**
Expand All @@ -56,9 +65,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);
}
}
Expand All @@ -75,16 +83,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("]");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down
6 changes: 6 additions & 0 deletions hadoop-project/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@
<yarnpkg.version>v1.22.5</yarnpkg.version>
<apache-ant.version>1.10.13</apache-ant.version>
<jmh.version>1.20</jmh.version>
<jnr.posix.version>3.1.19</jnr.posix.version>
</properties>

<dependencyManagement>
Expand Down Expand Up @@ -2135,6 +2136,11 @@
<artifactId>cache-api</artifactId>
<version>${cache.api.version}</version>
</dependency>
<dependency>
<groupId>com.github.jnr</groupId>
<artifactId>jnr-posix</artifactId>
<version>${jnr.posix.version}</version>
</dependency>
</dependencies>
</dependencyManagement>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,16 @@
<dependency>
<groupId>org.eclipse.jetty.websocket</groupId>
<artifactId>javax-websocket-server-impl</artifactId>
<exclusions>
<exclusion>
<artifactId>asm-commons</artifactId>
<groupId>org.ow2.asm</groupId>
</exclusion>
<exclusion>
<artifactId>asm-tree</artifactId>
<groupId>org.ow2.asm</groupId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>org.apache.hadoop.thirdparty</groupId>
Expand Down
Loading