8366298: FDLeakTest sometimes takes minutes to complete on Linux

[stefank]

While investigating JDK-8260555, which lowers the timeout factor from 4 to 1, we found that FDLeakTest sometimes times out on Linux.

The reason is that the test performs a fcntl call for each and every potential file descriptor number. This can be a large number of calls and sometimes results in minutes-long test executions.

I propose that we fix this by limiting the max number of open file descriptors. This lowers the test execution time to about 1 second.

The test has two processes. One that executes the libFDLeaker.c code below as an agent in the test JVM, then it forks into a exeFDLeakTester.c, which reads the property int max_fd = (int)sysconf(_SC_OPEN_MAX);. The setting of RLIMIT_NOFILE to 100 lowers max_fd to 100. I've verified this on both Linux and on macOS.

I've run the test manually on Linux and macOS and verified that it runs faster. I've also run this through tier1.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8366298: FDLeakTest sometimes takes minutes to complete on Linux (Bug - P4)

Reviewers

• Thomas Stuefe (@tstuefe - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/26979/head:pull/26979
$ git checkout pull/26979

Update a local copy of the PR:
$ git checkout pull/26979
$ git pull https://git.openjdk.org/jdk.git pull/26979/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 26979

View PR using the GUI difftool:
$ git pr show -t 26979

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/26979.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back stefank! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@stefank This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8366298: FDLeakTest sometimes takes minutes to complete on Linux

Reviewed-by: stuefe

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 35 new commits pushed to the master branch:

• 86d6a2e: 8366147: ZGC: ZPageAllocator::cleanup_failed_commit_single_partition may leak memory
• a2da75a: 8362884: [GCC static analyzer] unix NetworkInterface.c addif leak on early returns
• b8cdf31: 8365898: Specification of java.lang.module.ModuleDescriptor.packages() method can be improved
• ... and 32 more: https://git.openjdk.org/jdk/compare/88c39793670f2d36490530993feb60e138f43a70...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[openjdk]

@stefank The following label will be automatically applied to this pull request:

• core-libs

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 01: Full - Incremental (78fbf6d7)
• 00: Full (f83beb64)

[RogerRiggs]

@tstuefe Please comment if you have concerns about this change. Thanks

[tstuefe]

Thanks for looking at this. See comments inline.

[tstuefe]

Hmm, if the JVM already has opened > 100 fds at this point, we get difficult to understand errors. Any future open etc call will fail.

What I would do is to check, after the call to fopen() at line 39, check for errno==EMFILE. If that is true, write out a clear error message along the lines of "more than 100 files open?" (since that in itself would be worth to investigate; this little test should not open > 100 files, but who knows).

[stefank]

OK. I've added error logging for both setting the limit and opening the file. I didn't check specifically for EMFILE, but I hope that's fine?

[stefank]

I also verified that if I set the limit to 3 I do get the "Too many open files" error message.

[tstuefe]

Good, thanks!

[stefank]

Thanks for reviewing!