Skip to content

Java: Enhance java/jvm-exit query and add to quality #20190

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Napalys
Copy link
Contributor

@Napalys Napalys commented Aug 8, 2025

This PR integrates improvements from the java/jvm-exit-prevents-cleanup-and-reuse query in codeql-quality-queries into the main CodeQL repository. The enhanced query now provides better detection of problematic JVM termination calls with reduced false positives and has been promoted quality query suite.

It was decided to keep this query at medium precision, as there are quite a few false positives. This is largely due to the way command-line interfaces (CLI) and build tools handle application termination: they often use exit calls in ways that differ from standard software practices. As a result, the query may incorrectly flag these legitimate uses as potential issues, which impacts its overall accuracy.

Autofix appears feasible, though acceptance may vary depending on the specific case, as some instances could require more extensive refactoring within the repository.

Copy link
Contributor

github-actions bot commented Aug 8, 2025

QHelp previews:

java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToSystemExit.qhelp

Forcible JVM termination

Calling one of the methods System.exit, Runtime.halt, and Runtime.exit immediately terminates the Java Virtual Machine (JVM), effectively killing all threads without giving any of them a chance to perform cleanup actions or recover. As such, it is a dangerous thing to do: firstly, it can terminate the entire program inadvertently, and secondly, it can prevent important resources from being released or program state from being written to disk consistently.

It is sometimes considered acceptable to call System.exit from a program's main method in order to indicate the overall exit status of the program. The main method should be the primary place where exit conditions are handled, as it represents the natural termination point of the application. Such calls are an exception to this rule.

Recommendation

Instead of calling System.exit from non-main methods, prefer to propagate errors upward to the main method where they can be handled appropriately. Consider returning a special value (perhaps null) that users of the current method check for and recover from appropriately. Alternatively, throw a suitable exception, which unwinds the stack and allows properly written code to clean up after itself, while leaving other threads undisturbed. The main method can then catch these exceptions and decide whether to exit the program and with what exit code.

Example

In the following example, problem 1 shows that FileOutput.write tries to write some data to disk and terminates the JVM if this fails. This leaves the partially-written file on disk without any cleanup code running. It would be better to either return false to indicate the failure, or let the IOException propagate upwards and be handled by a method that knows how to recover.

Problem 2 is more subtle. In this example, there is just one entry point to the program (the main method), which constructs an Action and performs it. Action.run calls System.exit to indicate successful completion. Instead, the run method should return a status code or throw an exception on failure, allowing the main method to decide whether to exit and with what exit code. Consider how this code might be integrated in an application server that constructs Action instances and calls run on them without going through main. The fact that run terminates the JVM instead of returning its exit code makes that use-case impossible.

// Problem 1: Miss out cleanup code 
class FileOutput {
    boolean write(String[] s) {
        try {
            output.write(s.getBytes());
        } catch (IOException e) {
            System.exit(1); // BAD: Should handle or propagate error instead of exiting
        }
        return true;
    }
}

// Problem 2: Make code reuse difficult
class Action {
    public void run() {
        // ...
        // Perform tasks ...
        // ...
        System.exit(0); // BAD: Should return status or throw exception
    }
    public static void main(String[] args) {
        new Action().run();
    }
}

// Good example: Proper error handling
class BetterAction {
    public int run() throws Exception {
        // ...
        // Perform tasks ...
        // ...
        return 0; // Return status instead of calling System.exit
    }
    
    public static void main(String[] args) {
        try {
            BetterAction action = new BetterAction();
            int exitCode = action.run();
            System.exit(exitCode); // GOOD: Exit from main method
        } catch (Exception e) {
            System.err.println("Error: " + e.getMessage());
            System.exit(1); // GOOD: Exit from main method on error
        }
    }
}

References

@Napalys Napalys force-pushed the java/jvm-exit-query-promotion branch 4 times, most recently from 2cf0828 to 5a643be Compare August 11, 2025 06:48
@Napalys Napalys added the no-change-note-required This PR does not need a change note label Aug 11, 2025
@Napalys Napalys force-pushed the java/jvm-exit-query-promotion branch from 5a643be to f6aad96 Compare August 11, 2025 07:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Java no-change-note-required This PR does not need a change note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant