Skip to content

Commit

Permalink
Updated build.gradle - disable security manager for all Test Gradle "…
Browse files Browse the repository at this point in the history
…actions"

Created a "graceful exit" object + interceptor. It is supposed to replace System.Exit calls in different parts of the project;
Updated reporting.gradle by increasing versions of some dependencies;
Removed a DM_EXIT rule from spotbugs -> the calls of System.Exit must be replaced with GracefulExit.
The decision to create a custom interceptor was taken based on the discussion from Reddit: https://www.reddit.com/r/java/comments/1fpxmfp/jep_486_permanently_disable_the_security_manager/
  • Loading branch information
Vest committed Jan 11, 2025
1 parent 373f2b1 commit acd1482
Show file tree
Hide file tree
Showing 9 changed files with 236 additions and 7 deletions.
4 changes: 2 additions & 2 deletions PCGen-Formula/code/standards/spotbugs_ignore.xml
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
<Bug pattern="SE_NO_SERIALVERSIONID,SE_COMPARATOR_SHOULD_BE_SERIALIZABLE,SE_BAD_FIELD" />
</Match>
<Match>
<!-- not relevent to pcgen -->
<Bug pattern="IJU_SETUP_NO_SUPER,DP_DO_INSIDE_DO_PRIVILEGED,DM_EXIT" />
<!-- not relevant to pcgen -->
<Bug pattern="IJU_SETUP_NO_SUPER,DP_DO_INSIDE_DO_PRIVILEGED" />
</Match>
<Match>
<!-- we have other tools for code style -->
Expand Down
4 changes: 2 additions & 2 deletions PCGen-base/code/standards/spotbugs_ignore.xml
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
<Bug pattern="SE_NO_SERIALVERSIONID,SE_COMPARATOR_SHOULD_BE_SERIALIZABLE" />
</Match>
<Match>
<!-- not relevent to pcgen -->
<Bug pattern="IJU_SETUP_NO_SUPER,DP_DO_INSIDE_DO_PRIVILEGED,DM_EXIT" />
<!-- not relevant to pcgen -->
<Bug pattern="IJU_SETUP_NO_SUPER,DP_DO_INSIDE_DO_PRIVILEGED" />
</Match>
<Match>
<!-- we have other tools for code style -->
Expand Down
1 change: 1 addition & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -852,6 +852,7 @@ allprojects {
'-Dprism.order=sw',
'-Dprism.verbose=true',
'-Djavafx.macosx.embedded=true',
'-Djava.security.manager=disallow',

"--module-path", layout.projectDirectory.dir("mods/lib"),
"--add-modules", "javafx.controls,javafx.web,javafx.swing,javafx.fxml,javafx.graphics",
Expand Down
4 changes: 2 additions & 2 deletions code/gradle/reporting.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ checkstyle {
configFile = new File('code/standards/checkstyle.xml')
configProperties = [samedir: "${rootDir}/code/standards"]
showViolations = true
toolVersion = '10.20.1'
toolVersion = '10.21.1'
sourceSets = []
}

Expand All @@ -19,7 +19,7 @@ pmd {
ruleSetFiles = files('code/standards/ruleset.xml')
consoleOutput = true
sourceSets = []
toolVersion = "7.7.0"
toolVersion = "7.9.0"
incrementalAnalysis = true
}

Expand Down
7 changes: 7 additions & 0 deletions code/src/testcommon/util/ExitFunction.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package util;

@FunctionalInterface
public interface ExitFunction
{
void exit(int status);
}
7 changes: 7 additions & 0 deletions code/src/testcommon/util/ExitInterceptor.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package util;

@FunctionalInterface
public interface ExitInterceptor
{
boolean intercept(int status);
}
98 changes: 98 additions & 0 deletions code/src/testcommon/util/GracefulExit.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
package util;

import java.text.MessageFormat;
import java.util.ArrayList;
import java.util.List;
import java.util.function.Predicate;
import java.util.logging.Level;
import java.util.logging.Logger;

public class GracefulExit
{
private static final Logger LOG = Logger.getLogger(GracefulExit.class.getName());

/* This lock protects the preceding static fields */
private static class Lock
{
}

private static final Object lock = new GracefulExit.Lock();

private static final List<ExitInterceptor> interceptors = new ArrayList<>();

private static boolean isShuttingDown = false;

private static ExitFunction exitFunction = System::exit;

static void exit(int status)
{
synchronized (lock)
{
LOG.log(Level.FINEST, MessageFormat.format(
"Started exiting with the status {0}. There are {1} interceptors.", status,
interceptors.size()));
isShuttingDown = true;

boolean shouldExit = interceptors.stream()
.map(interceptor -> interceptor.intercept(status))
.peek(intercepted -> LOG.log(Level.FINEST,
MessageFormat.format("Intercepted exit: {0} from registered interceptor: {1}",
intercepted, intercepted.getClass().getName())))
.filter(Predicate.isEqual(Boolean.FALSE))
.findAny()
.orElse(Boolean.TRUE);

try
{
if (shouldExit)
{
LOG.info("Exiting gracefully with status " + status);
exitFunction.exit(status);
}
} finally
{
isShuttingDown = false;
}
}
}

public static void addExitInterceptor(ExitInterceptor interceptor)
{
synchronized (lock)
{
if (isShuttingDown)
{
throw new IllegalStateException("Cannot add an interceptor after shutdown has started");
}
if (interceptors.contains(interceptor))
{
throw new IllegalArgumentException("Interceptor already registered");
}
interceptors.add(interceptor);
}
}

public static void clearExitInterceptors()
{
synchronized (lock)
{
if (isShuttingDown)
{
throw new IllegalStateException("Cannot clear interceptors after shutdown has started");
}
interceptors.clear();
}
}

public static void registerExitFunction(ExitFunction exitFunction)
{
synchronized (lock)
{
if (isShuttingDown)
{
throw new IllegalStateException("Cannot register an exit function after shutdown has started");
}
GracefulExit.exitFunction = exitFunction;
}
}
}
116 changes: 116 additions & 0 deletions code/src/testcommon/util/GracefulExitTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
package util;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import java.util.concurrent.atomic.AtomicInteger;

import static org.junit.jupiter.api.Assertions.*;

class GracefulExitTest
{

public static final ExitFunction EXIT_FUNCTION_WITH_EXCEPTION = status -> {
throw new RuntimeException("Test exception");
};

public static final ExitInterceptor EMPTY_EXIT_INTERCEPTOR = status -> true;

@BeforeEach
void setUp()
{
GracefulExit.registerExitFunction(status -> { /* do nothing */ });
GracefulExit.clearExitInterceptors();
}

@Test
void testDefaultExit()
{
GracefulExit.registerExitFunction(status -> {
assertEquals(23, status, "Status code should be 23");
});
GracefulExit.exit(23);
}

@Test
void testExitWithThrownException() {
GracefulExit.registerExitFunction(EXIT_FUNCTION_WITH_EXCEPTION);
assertThrows(RuntimeException.class, () -> GracefulExit.exit(0), "Should throw an exception");
}

@Test
void testNoExitWithExceptionThrown() {
GracefulExit.registerExitFunction(EXIT_FUNCTION_WITH_EXCEPTION);
GracefulExit.addExitInterceptor(status -> false);
assertDoesNotThrow(() -> GracefulExit.exit(0), "Should not throw an exception because the exit was intercepted");
}

@Test
void testUniqueInterceptor()
{
ExitInterceptor customInterceptor = status -> false;
GracefulExit.addExitInterceptor(customInterceptor);

assertThrows(IllegalArgumentException.class, () -> {
GracefulExit.addExitInterceptor(customInterceptor);
}, "Should throw an exception because the interceptor is already registered");
}

@Test
void testAddHookDuringExit()
{
GracefulExit.registerExitFunction(status -> {});

GracefulExit.addExitInterceptor(status -> {
assertThrows(IllegalStateException.class, () -> {
GracefulExit.addExitInterceptor(EMPTY_EXIT_INTERCEPTOR);
}, "Should throw an exception because the interceptor is added during exit");
return true;
});
GracefulExit.exit(0);
}

@Test
void testAddExitDuringExit()
{
GracefulExit.registerExitFunction(status -> {
assertThrows(IllegalStateException.class, () -> {
GracefulExit.registerExitFunction(EXIT_FUNCTION_WITH_EXCEPTION);
}, "Should throw an exception because the exit function is added during exit");
});
GracefulExit.exit(0);
}

@Test
void testClearHooksDuringExit()
{
GracefulExit.registerExitFunction(status -> {
assertThrows(IllegalStateException.class, GracefulExit::clearExitInterceptors, "Should throw an exception because the interceptors are cleared during exit");
});
GracefulExit.exit(0);
}

@Test
void testExitHookSequence()
{
AtomicInteger count = new AtomicInteger(0);

GracefulExit.addExitInterceptor(status -> {
assertEquals(0, count.get(), "The first interceptor should be called first");
count.incrementAndGet();
return true;
});
GracefulExit.addExitInterceptor(status -> {
assertEquals(1, count.get(), "The second interceptor should be called next");
count.incrementAndGet();
return true;
});
GracefulExit.addExitInterceptor(status -> {
assertEquals(2, count.get(), "The last interceptor should be called next");
count.incrementAndGet();
return true;
});
GracefulExit.exit(0);
assertEquals(3, count.get(), "All interceptors should have been called");
}
}
2 changes: 1 addition & 1 deletion code/standards/spotbugs_ignore.xml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
</Match>
<Match>
<!-- not relevant to pcgen -->
<Bug pattern="IJU_SETUP_NO_SUPER,DP_DO_INSIDE_DO_PRIVILEGED,DM_EXIT,SE_NO_SERIALVERSIONID"/>
<Bug pattern="IJU_SETUP_NO_SUPER,DP_DO_INSIDE_DO_PRIVILEGED,SE_NO_SERIALVERSIONID"/>
</Match>
<Match>
<!-- we have other tools for code style -->
Expand Down

0 comments on commit acd1482

Please sign in to comment.