On Fri, 26 Jan 2024 14:52:49 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
> Can I please get a review of this change which proposes to address > https://bugs.openjdk.org/browse/JDK-8324668? > > The JVM can be launched with the `jdwp` agent, like > `-agentlib:jdwp=transport=xxx,server=y`. Among other options to the `jdwp` > agentlib, one option is the `launch` option, which is of the form > `launch=<cmdline>`. `cmdline` can be any arbitrary command. > > When that option is set, `jdwp` during initialization will then launch that > command. The implementation of launching the command (on *nix) involves > `fork()` followed by `execvp()`. After the child process is forked, jdwp > implementation, closes the file descriptors that were copied over (by fork()) > from the parent (JVM) process into the (arbitrary) child process. Since jdwp > doesn't know how many such file descriptors exist, it queries a system level > configuration for that launched process to know max allowed file descriptors > (`OPEN_MAX`). It then iterates over each of these file descriptor and calls > close() on them sequentially one at a time, irrespective of whether or not > those many file descriptors were indeed present or not. > > Until recently, on macOS, for the JVM we used to limit the max file > descriptors to 10240. The jdwp code above would then iterate over these 10240 > file descriptors (I'm leaving out the part where the code skips the standard > input/output/error file descriptors) and close them one at a time (even if > there weren't that many). This apparently would finish in reasonable amount > of time (likely slow, but wasn't noticed to cause any issues). > > In https://bugs.openjdk.org/browse/JDK-8300088 we stopped setting that 10240 > limit on macOS for the JVM. After that change, on macos, the max file > descriptors for JVM turns out to be 2147483647. When jdwp then launches the > child process from the JVM, it then has to now iterate almost 2147483647 > times and close each of the file descriptors in that logic. This now became > extremely slow and noticable and caused 3 tests > (https://bugs.openjdk.org/browse/JDK-8324578) which were using the > `launch=<cmdline>` option to start failing with a timeout - the jdwp process > hadn't yet finished the iteration to close the file descriptors for the child > process, by the time jtreg noticed the test had gone past the allowed timeout. > > In theory, this entire code which deals with launching a child process from > the JVM is what the native implementation of Java's > `java.lang.ProcessBuilder.start()` might do. So l had a look at how it's > handled there and it turns out, t... The PR description appears in *every* email sent for the PR. For PRs with extensive descriptions, perhaps they can be included as a separate comment. ------------- PR Comment: https://git.openjdk.org/jdk/pull/17588#issuecomment-1912212785
