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

Improve container detection if current process is pid 1 #1322

Closed
wants to merge 1 commit into from

Conversation

maxxedev
Copy link
Contributor

@maxxedev maxxedev commented Nov 30, 2024

If current process is pid 1, read directly from System.getenv() instead of performing file i/o on /proc/1/environ

  • Read the contribution guidelines for this project.
  • Run a successful build using the default Maven goal with mvn; that's mvn on the command line by itself.
  • Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied. This may not always be possible but is a best-practice.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Each commit in the pull request should have a meaningful subject line and body. Note that commits might be squashed by a maintainer on merge.

@ppkarwasz
Copy link

@maxxedev,

Can you explain in what consists the improvement? AFAIK the JVM will never have PID 1. Even if it does, checking /proc/1/environ and the environment of the JVM is equivalent.

@maxxedev
Copy link
Contributor Author

@ppkarwasz

JVM will never have PID 1

JVM can certainly be PID 1 inside a container. Example:

$ cat App.java
public class App {

        public static void main(String... args) {
                var mxBean = java.lang.management.ManagementFactory.getRuntimeMXBean();
                System.out.println(mxBean.getName());
        }
}

$ docker run -it --entrypoint java -v `pwd`:/tmp/ openjdk:21 /tmp/App.java
1@afecfa5e25bb

 

checking /proc/1/environ and the environment of the JVM is equivalent.

Yes, equivalent but perhaps reading from System.getenv() is a slight improvement by avoiding file i/o on procfs

@ppkarwasz
Copy link

JVM will never have PID 1

JVM can certainly be PID 1 inside a container. Example:

Point taken.

checking /proc/1/environ and the environment of the JVM is equivalent.

Yes, equivalent but perhaps reading from System.getenv() is a slight improvement by avoiding file i/o on procfs

Your code requires the java.management Java module to handle a specific case that was already handled correctly by the previous code (which only requires java.base). IMHO this is not an improvement.

@maxxedev maxxedev closed this Dec 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants