8298619: java/io/File/GetXSpace.java is failing #12397

bplb · 2023-02-03T02:02:39Z

Modify the Space instances used for size comparison to be created with total number of bytes derived from the Windows diskFree utility instead of Cygwin’s df.

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-8298619: java/io/File/GetXSpace.java is failing

Reviewers

Roger Riggs (@RogerRiggs - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 12397

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

Using diff file

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

Webrev

Link to Webrev Comment

bridgekeeper · 2023-02-03T02:04:06Z

👋 Welcome back bpb! 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.

bplb · 2023-02-03T02:04:30Z

With reference to the Windows DISK_SPACE_INFORMATION structure, the File methods getTotalSpace, getFreeSpace, and getUsableSpace return the number of bytes implied by CallerTotalAllocationUnits, ActualAvailableAllocationUnits, and CallerAvailableAllocationUnits, respectively. The total and usable values reflect any quota imposed on the user. Currently the reference values against which these space values are compared are derived from the Cygwin df command. The total number of bytes given by df corresponds to ActualTotalAllocationUnits and does not account for quotas. Hence it is not appropriate as a reference when quotas are imposed. The total number of bytes indicated by the output of the Windows diskFree utility is however equivalent to CallerTotalAllocationUnits hence is the correct reference.

With this change the test passes on all platforms, include 110 repeats on Windows.

openjdk · 2023-02-03T02:06:30Z

@bplb 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 · 2023-02-03T02:12:43Z

Webrevs

AlanBateman · 2023-02-10T13:51:24Z

test/jdk/java/io/File/GetXSpace.java

@@ -149,6 +149,37 @@ private static void diskFree() throws IOException {
        out.println(sb);
    }

+    private static String getCallerTotalSpace(String name) throws IOException {


It seems a bit fragile to be parsing the output of fsutil volume diskFree as the output seems to vary by Windows releases and maybe configuration. So minimally, I think it should be changed to use ProcessTools so that the command and the output show up in the .jtr file.

In passing, you might want to choose a different method name to make it clearer what it does, maybe volumeDiskFree?

It seems a bit fragile to be parsing the output of fsutil volume diskFree as the output seems to vary by Windows releases and maybe configuration. So minimally, I think it should be changed to use ProcessTools so that the command and the output show up in the .jtr file.

In passing, you might want to choose a different method name to make it clearer what it does, maybe volumeDiskFree?

Another possibility would be to add a native method to the test itself to invoke GetDiskSpaceInformationW to obtain the value of CallerTotalAllocationUnits (in bytes) from the DISK_SPACE_INFORMATION structure.

Another possibility would be to add a native method to the test itself to invoke GetDiskSpaceInformationW to obtain the value of CallerTotalAllocationUnits (in bytes) from the DISK_SPACE_INFORMATION structure.

That would be more reliable than parsing the output of fsutil volume so worth trying. You might have to dig into which of these win32 works with quotas as that seems to be part of the issue. The JDK uses GetDiskFreeSpaceExW. Also this test might have to do a few iterations so that it captures free space info while the system is in quiescence - I suspect some of the reliability issues has been concurrent activity that changes the volume space usage significantly while this test is running.

There has definitely been a problem with quotas on Windows. I set up quotas on actual Windows 11 hardware and replicated the same failure. I am not sure how much lack of system quiescence is to blame, but there would be no harm in building in some robustness for lack of quiescence while we're at it.

Spawning df and diskFree processes have been replaced with native calls. For a reason as yet undetermined, the Windows function GetDiskSpaceInformationW fails to load on Windows Server 2016 so it is loaded dynamically and, if not found, a workaround is used.

Testing in the CI against the latest commit 114857c has passed more than 500 iterations on Windows with no failures.

This test has been run successfully hundreds of times on CI system nodes where it previously failed.

…al size is estimated instead of exact

bridgekeeper · 2023-03-29T01:59:31Z

@bplb This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

bplb · 2023-03-29T15:09:20Z

This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity.

Ping.

test/jdk/java/io/File/libGetXSpace.c

test/jdk/java/io/File/GetXSpace.java

RogerRiggs

LGTM

openjdk · 2023-04-03T16:52:38Z

@bplb 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:

8298619: java/io/File/GetXSpace.java is failing

Reviewed-by: rriggs

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 459 new commits pushed to the master branch:

b062b1b: 8304743: Compile_lock and SystemDictionary updates
df819cf: 8304945: StringBuilder and StringBuffer should implement Appendable explicitly
312bbe7: 8305485: Problemlist runtime/Thread/TestAlwaysPreTouchStacks.java
50e31e0: 8305442: (bf) Direct and view implementations of CharBuffer.toString(int, int) do not need to catch SIOBE
85e3974: 8304014: Convert test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java to junit
40aea04: 8278268: (ch) InputStream returned by Channels.newInputStream should have fast path for FileChannel targets
9b9b5a7: 8302323: Add repeat methods to StringBuilder/StringBuffer
dd7ca75: 8305478: [REDO] disable gtest/NMTGtests.java sub-tests failing due to JDK-8305414
f9827ad: 8288109: HttpExchangeImpl.setAttribute does not allow null value after JDK-8266897
6010de0: 8305417: disable gtest/NMTGtests.java sub-tests failing due to JDK-8305414
... and 449 more: https://git.openjdk.org/jdk/compare/b527edd3388ad6a0d44a291983b08b2b5c023f8f...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.

bplb · 2023-04-04T15:54:45Z

/integrate

openjdk · 2023-04-04T15:57:06Z

Going to push as commit 3ef834f.
Since your change was applied there have been 475 commits pushed to the master branch:

c594119: 8305511: Remove ignore from com/sun/jdi/PopAndInvokeTest.java
62bd2eb: 8303147: [s390x] fast & slow debug builds are broken
f9ed825: 8303082: [AIX] Missing C++ name demangling with XLClang++
73a084c: 8305504: stutter typo in java.compiler files
7b92210: 8305525: Problemlist runtime/ErrorHandling/TestDwarf on x86
5881b09: 8305500: SystemDictionary::find_placeholder/find_class declared but not used
7ca2aec: 8304033: JFR: Missing thread
ac01e24: 8305242: Remove non-invariant assert(EventThreadDump::is_enabled())
a663d5d: 8305520: ToggleNotifyJvmtiTest.java fails with release VMs
24c6af0: 8173709: Fix VerifyLoopOptimizations - step 1 - minimal infrastructure
... and 465 more: https://git.openjdk.org/jdk/compare/b527edd3388ad6a0d44a291983b08b2b5c023f8f...master

Your commit was automatically rebased without conflicts.

openjdk · 2023-04-04T15:57:38Z

@bplb Pushed as commit 3ef834f.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

turbanoff · 2023-09-04T12:13:40Z

It seems after refactoring test started to fail on my Win11 VM.

STDOUT:
--- Testing volumes
Total bytes                     : 299,877,003,264 (279.3 GB)
Total quota free bytes          : 202,708,971,520 (188.8 GB)
Unavailable pool bytes          :               0 (  0.0 KB)
Quota unavailable pool bytes    :               0 (  0.0 KB)
Used bytes                      :  92,833,345,536 ( 86.5 GB)
Total Reserved bytes            :   4,334,686,208 (  4.0 GB)
Volume storage reserved bytes   :   4,296,867,840 (  4.0 GB)
Available committed bytes       :               0 (  0.0 KB)
Pool available bytes            :               0 (  0.0 KB)


SecurityManager = null
C:\ (299877003264):
  getSpace0 total = 299877003264 free = 202708967424 usable = 202708967424
  getXSpace total = 299877003264 free = 202708967424 usable = 202708967424
STDERR:
WARNING: A terminally deprecated method in java.lang.System has been called
WARNING: System::setSecurityManager has been called by GetXSpace (file:/C:/Projects/jdk/build/windows-x86_64-server-slowdebug/test-support/jtreg_test_jdk_java_io_File/classes/2/java/io/File/GetXSpace.d/)
WARNING: Please consider reporting this to the maintainers of GetXSpace
WARNING: System::setSecurityManager will be removed in a future release
java.lang.RuntimeException: The parameter is incorrect
        at GetXSpace.getSpace0(Native Method)
        at GetXSpace$Space.<init>(GetXSpace.java:112)
        at GetXSpace.testVolumes(GetXSpace.java:431)
        at GetXSpace.main(GetXSpace.java:469)
        at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
        at java.base/java.lang.reflect.Method.invoke(Method.java:580)
        at com.sun.javatest.regtest.agent.MainWrapper$MainTask.run(MainWrapper.java:138)
        at java.base/java.lang.Thread.run(Thread.java:1570)

JavaTest Message: Test threw exception: java.lang.RuntimeException: The parameter is incorrect

Fails on disk D:

Call getSpace0 for D:\
GetDiskSpaceInformationW failed

If I eject disk, I got fail too:

java.lang.RuntimeException: The device is not ready
        at GetXSpace.getSpace0(Native Method)
        at GetXSpace$Space.<init>(GetXSpace.java:113)
        at GetXSpace.testVolumes(GetXSpace.java:432)
        at GetXSpace.main(GetXSpace.java:470)
        at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
        at java.base/java.lang.reflect.Method.invoke(Method.java:580)
        at com.sun.javatest.regtest.agent.MainWrapper$MainTask.run(MainWrapper.java:138)
        at java.base/java.lang.Thread.run(Thread.java:1570)

mlbridge · 2023-09-05T21:20:22Z

Mailing list message from Brian Burkhalter on core-libs-dev:

The changes to this test were developed on a native Windows 11 machine. I have never seen this problem.

A web search suggests that this is a Windows error message possibly due to drive error.

On Sep 4, 2023, at 5:16 AM, Andrey Turbanov <aturbanov at openjdk.org<mailto:aturbanov at openjdk.org>> wrote:

It seems after refactoring test started to fail on my Win11 VM.

[?]
java.lang.RuntimeException: The parameter is incorrect
at GetXSpace.getSpace0(Native Method)
at GetXSpace$Space.<init>(GetXSpace.java:112)

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/core-libs-dev/attachments/20230905/dce5c39d/attachment.htm>

8298619: java/io/File/GetXSpace.java is failing

9eef8cd

openjdk bot added the rfr Pull request is ready for review label Feb 3, 2023

openjdk bot added the core-libs core-libs-dev@openjdk.org label Feb 3, 2023

AlanBateman reviewed Feb 10, 2023

View reviewed changes

bplb added 4 commits February 22, 2023 08:54

8298619: Replace df and diskFree with native calls

2c41b5b

8298619: Load GetDiskSpaceInformationW dynamically

42ad4ce

Merge

f1da253

8298619: Change getSpace0() to return a boolean; print warning if tot…

114857c

…al size is estimated instead of exact

RogerRiggs reviewed Mar 29, 2023

View reviewed changes

test/jdk/java/io/File/libGetXSpace.c Show resolved Hide resolved

test/jdk/java/io/File/GetXSpace.java Outdated Show resolved Hide resolved

8305157: Clean up

9ae317e

RogerRiggs approved these changes Apr 3, 2023

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Apr 3, 2023

openjdk bot added the integrated Pull request has been integrated label Apr 4, 2023

openjdk bot closed this Apr 4, 2023

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Apr 4, 2023

bplb deleted the File-GetXSpace-8298619 branch April 5, 2023 21:11

sxa mentioned this pull request Nov 13, 2023

8319958: test/jdk/java/io/File/libGetXSpace.c does not compile on Windows 32-bit #16628

Closed

3 tasks

kurashige23 mentioned this pull request Oct 31, 2024

8343342: java/io/File/GetXSpace.java fails on Windows with CD-ROM drive #21799

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8298619: java/io/File/GetXSpace.java is failing #12397

8298619: java/io/File/GetXSpace.java is failing #12397

bplb commented Feb 3, 2023 •

edited by openjdk bot

Loading

bridgekeeper bot commented Feb 3, 2023

bplb commented Feb 3, 2023 •

edited

Loading

openjdk bot commented Feb 3, 2023

mlbridge bot commented Feb 3, 2023 •

edited

Loading

AlanBateman Feb 10, 2023

bplb Feb 10, 2023

AlanBateman Feb 12, 2023

bplb Feb 14, 2023

bplb Feb 22, 2023

bplb Feb 28, 2023

bplb Apr 3, 2023

bridgekeeper bot commented Mar 29, 2023

bplb commented Mar 29, 2023

RogerRiggs left a comment

openjdk bot commented Apr 3, 2023 •

edited

Loading

bplb commented Apr 4, 2023

openjdk bot commented Apr 4, 2023

openjdk bot commented Apr 4, 2023

turbanoff commented Sep 4, 2023 •

edited

Loading

mlbridge bot commented Sep 5, 2023

8298619: java/io/File/GetXSpace.java is failing #12397

8298619: java/io/File/GetXSpace.java is failing #12397

Conversation

bplb commented Feb 3, 2023 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

Webrev

bridgekeeper bot commented Feb 3, 2023

bplb commented Feb 3, 2023 • edited Loading

openjdk bot commented Feb 3, 2023

mlbridge bot commented Feb 3, 2023 • edited Loading

Webrevs

AlanBateman Feb 10, 2023

Choose a reason for hiding this comment

bplb Feb 10, 2023

Choose a reason for hiding this comment

AlanBateman Feb 12, 2023

Choose a reason for hiding this comment

bplb Feb 14, 2023

Choose a reason for hiding this comment

bplb Feb 22, 2023

Choose a reason for hiding this comment

bplb Feb 28, 2023

Choose a reason for hiding this comment

bplb Apr 3, 2023

Choose a reason for hiding this comment

bridgekeeper bot commented Mar 29, 2023

bplb commented Mar 29, 2023

RogerRiggs left a comment

Choose a reason for hiding this comment

openjdk bot commented Apr 3, 2023 • edited Loading

bplb commented Apr 4, 2023

openjdk bot commented Apr 4, 2023

openjdk bot commented Apr 4, 2023

turbanoff commented Sep 4, 2023 • edited Loading

mlbridge bot commented Sep 5, 2023

bplb commented Feb 3, 2023 •

edited by openjdk bot

Loading

bplb commented Feb 3, 2023 •

edited

Loading

mlbridge bot commented Feb 3, 2023 •

edited

Loading

openjdk bot commented Apr 3, 2023 •

edited

Loading

turbanoff commented Sep 4, 2023 •

edited

Loading