8254350: CompletableFuture.get may swallow InterruptedException

[Martin-Buchholz]

/cc core-libs

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed

Issue

• JDK-8254350: CompletableFuture.get may swallow InterruptedException

Reviewers

• Alan Bateman (@AlanBateman - Reviewer)

Download

$ git fetch https://git.openjdk.java.net/jdk pull/1651/head:pull/1651
$ git checkout pull/1651

[Martin-Buchholz]

/issue add JDK-8254350

[bridgekeeper]

👋 Welcome back martin! 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]

@Martin-Buchholz
The core-libs label was successfully added.

[openjdk]

@Martin-Buchholz This issue is referenced in the PR title - it will now be updated.

[mlbridge]

Webrevs

• 01: Full - Incremental (762a171)
• 00: Full (4e32f8c1321892a004eed43227b2746121ca6a1c)

[AlanBateman]

Do you want the "TODO" in the commit?

[openjdk]

@Martin-Buchholz 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:

8254350: CompletableFuture.get may swallow InterruptedException

Reviewed-by: alanb

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 1 new commit pushed to the master branch:

• 500ab45: 8257769: Cipher.getParameters() throws NPE for ChaCha20-Poly1305

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
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.

[dfuch]

Hi Martin,

Is using a ThreadLocalRandom important for the test logic?
I was wondering whether it would be better to use ( * @library /test/lib) jdk.test.lib.RandomFactory,
which prints the random seed in the output so that you can reproduce with the same pseudo-random
sequence in case of test failures.

best regards,

-- daniel

[AlanBateman]

RandomFactory is probably overkill here as the test just needs to exercise untimed and timed get. So just a random boolean rather than a wide range of random values.

[dfuch]

OK.

[mlbridge]

Mailing list message from Martin Buchholz on core-libs-dev:

On Mon, Dec 7, 2020 at 11:56 PM Alan Bateman <alanb at openjdk.java.net> wrote:

37: // TODO: Rewrite as a CompletableFuture tck test ?

Do you want the "TODO" in the commit?

Yes!

[mlbridge]

Mailing list message from Martin Buchholz on core-libs-dev:

On Tue, Dec 8, 2020 at 3:54 AM Daniel Fuchs <dfuchs at openjdk.java.net> wrote:

RandomFactory is probably overkill here as the test just needs to
exercise untimed and timed get. So just a random boolean rather than a wide
range of random values.

OK.

There's a conflict between the desires to do more thorough testing and
avoid flaky intermittent failures.

Especially in j.u.concurrent we embrace test non-determinism, much of it
comes from the concurrency we're testing, and retrieving a random seed for
reproducibility is not worth the effort.

No one does a good job of race prevention through testing.

[Martin-Buchholz]

This change went into jdk16 instead and was automatically forward-ported to jdk, so abandoning this PR.