-
Notifications
You must be signed in to change notification settings - Fork 3.3k
HBASE-27594 [JDK17] SecurityManager is deprecated since JDK17 #6932
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
/* | ||
* Licensed to the Apache Software Foundation (ASF) under one | ||
* or more contributor license agreements. See the NOTICE file | ||
* distributed with this work for additional information | ||
* regarding copyright ownership. The ASF licenses this file | ||
* to you under the Apache License, Version 2.0 (the | ||
* "License"); you may not use this file except in compliance | ||
* with the License. You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package org.apache.hadoop.hbase.util; | ||
|
||
import org.apache.yetus.audience.InterfaceAudience; | ||
|
||
/** | ||
* A utility class to wrap system exit calls. We use this to allow for testing and to provide a | ||
* consistent way to handle system exit across the codebase. NOTE: We should avoid calling | ||
* System.exit directly in the codebase. Instead, we should use this class to handle system exit | ||
* calls. | ||
*/ | ||
@InterfaceAudience.Private | ||
public class ExitHandler { | ||
private static ExitHandler instance = new ExitHandler(); | ||
|
||
public static ExitHandler getInstance() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should I wrap into Double-Checked Locking. seems overkill for this use case. WDYT? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some kind of semaphore to ensure that only one thread in a JVM holds a test instance ? |
||
return instance; | ||
} | ||
|
||
public static void setInstance(ExitHandler handler) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should i let this be called from only tests? |
||
instance = handler; | ||
} | ||
|
||
/** | ||
* This method is called when the system needs to exit. By default, it calls System.exit. | ||
* Subclasses can override this method to provide custom exit behavior. | ||
* @param status the exit status | ||
*/ | ||
@SuppressWarnings("checkstyle:RegexpSinglelineJava") | ||
public void exit(int status) { | ||
System.exit(status); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,40 +17,46 @@ | |
*/ | ||
package org.apache.hadoop.hbase; | ||
|
||
import org.apache.hadoop.hbase.util.ExitHandler; | ||
import org.junit.rules.TestRule; | ||
import org.junit.runner.Description; | ||
import org.junit.runners.model.Statement; | ||
|
||
/* | ||
* Test rules to prevent System.exit or Runtime.halt from exiting | ||
* the JVM - instead an exception is thrown. | ||
* */ | ||
/** | ||
* Test rules to prevent Exit calls in tests. This is useful to prevent tests from exiting the JVM | ||
* and to ensure that tests can be run in a controlled environment. This rule replaces the default | ||
* ExitHandler with a mock handler that throws an exception when exit is called. | ||
*/ | ||
public class SystemExitRule implements TestRule { | ||
final static SecurityManager securityManager = new TestSecurityManager(); | ||
private static final ExitHandler mockHandler = new ExitHandler() { | ||
@Override | ||
public void exit(int status) { | ||
throw new SystemExitInTestException(); | ||
} | ||
}; | ||
|
||
@Override | ||
public Statement apply(final Statement s, Description d) { | ||
return new Statement() { | ||
@Override | ||
public void evaluate() throws Throwable { | ||
|
||
ExitHandler originalHandler = ExitHandler.getInstance(); | ||
try { | ||
forbidSystemExitCall(); | ||
s.evaluate(); | ||
} finally { | ||
System.setSecurityManager(null); | ||
ExitHandler.setInstance(originalHandler); | ||
} | ||
} | ||
|
||
}; | ||
}; | ||
} | ||
|
||
// Exiting the JVM is not allowed in tests and this exception is thrown instead | ||
// when it is done | ||
public static class SystemExitInTestException extends SecurityException { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we still throw SecurityException? |
||
} | ||
|
||
private static void forbidSystemExitCall() { | ||
System.setSecurityManager(securityManager); | ||
ExitHandler.setInstance(mockHandler); | ||
} | ||
} |
This file was deleted.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just FYI: added this checkstyle rule to raise checkstyle an issue if anyone tries to directly use System.exit