Skip to content

8359919: Minor java.util.concurrent doc improvements #25880

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

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

DougLea
Copy link
Contributor

@DougLea DougLea commented Jun 18, 2025

This collects miscellaneous open issues that can be resolved with documentation updates; each indicated by adding JDK issue numbers


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

Issues

  • JDK-8359919: Minor java.util.concurrent doc improvements (Bug - P4)
  • JDK-8187775: AtomicReferenceFieldUpdater does not support static fields (Bug - P4)
  • JDK-8254060: SubmissionPublisher close hangs if a publication is pending (Bug - P4)
  • JDK-8210149: Example in JavaDoc for java.util.concurrent.Flow violates Reactive Streams spec (Enhancement - P4)
  • JDK-8199501: Improve documentation of CompletableFuture, CompletionStage (Enhancement - P4)
  • JDK-8233050: CompletableFuture whenComplete and thenApply change exceptional result (Bug - P4)
  • JDK-8210312: JavaDoc example in SubmissionPublisher will potentially crash (Enhancement - P4)
  • JDK-8292365: CompletableFuture and CompletionStage should document Memory Model guarantees (Enhancement - P4)
  • JDK-8356304: Define "enabled" in ScheduledExecutorService (Bug - P4)
  • JDK-8353155: FutureTask#run(): doc implies synchronous, implementation is async (Enhancement - P4)
  • JDK-8186959: Clarify that Executors.newScheduledThreadPool() is fixed-size (Bug - P3)
  • JDK-8190889: TimeUnit.wait should document IllegalMonitorStateException (Bug - P4)
  • JDK-6351533: CyclicBarrier reset() should return the number of awaiters (Enhancement - P4)
  • JDK-6317534: CyclicBarrier should have a cancel() method (Enhancement - P3)
  • JDK-8195628: Documentation for lock(), trylock(), lockInterruptibly​() of ReentrantReadWriteLock.WriteLock needs to be corrected (Bug - P4)
  • JDK-8333172: Document a recommendation to use VarHandles instead of java.util.concurrent.atomic.*FieldUpdater (Enhancement - P4)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 25880

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

Using diff file

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

Using Webrev

Link to Webrev Comment

@DougLea
Copy link
Contributor Author

DougLea commented Jun 18, 2025

/issue JDK-8187775

@DougLea
Copy link
Contributor Author

DougLea commented Jun 18, 2025

/issue JDK-8254060

@bridgekeeper
Copy link

bridgekeeper bot commented Jun 18, 2025

👋 Welcome back dl! 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
Copy link

openjdk bot commented Jun 18, 2025

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

@DougLea
Copy link
Contributor Author

DougLea commented Jun 18, 2025

/issue JDK-8210149

@openjdk openjdk bot added the rfr Pull request is ready for review label Jun 18, 2025
@DougLea
Copy link
Contributor Author

DougLea commented Jun 18, 2025

/issue JDK-8199501

@openjdk
Copy link

openjdk bot commented Jun 18, 2025

@DougLea
Adding additional issue to issue list: 8187775: AtomicReferenceFieldUpdater does not support static fields.

@DougLea
Copy link
Contributor Author

DougLea commented Jun 18, 2025

/issue JDK-8233050

@openjdk
Copy link

openjdk bot commented Jun 18, 2025

@DougLea
Adding additional issue to issue list: 8254060: SubmissionPublisher close hangs if a publication is pending.

@DougLea
Copy link
Contributor Author

DougLea commented Jun 18, 2025

/issue JDK-8210312

@DougLea
Copy link
Contributor Author

DougLea commented Jun 18, 2025

/issue JDK-8292365

@openjdk
Copy link

openjdk bot commented Jun 18, 2025

@DougLea
Adding additional issue to issue list: 8210149: Example in JavaDoc for java.util.concurrent.Flow violates Reactive Streams spec.

@DougLea
Copy link
Contributor Author

DougLea commented Jun 18, 2025

/issue JDK-8356304

@openjdk
Copy link

openjdk bot commented Jun 18, 2025

@DougLea
Adding additional issue to issue list: 8199501: Improve documentation of CompletableFuture, CompletionStage.

@openjdk
Copy link

openjdk bot commented Jun 18, 2025

@DougLea
Adding additional issue to issue list: 8233050: CompletableFuture whenCompleteandthenApply change exceptional result.

@openjdk
Copy link

openjdk bot commented Jun 18, 2025

@DougLea
Adding additional issue to issue list: 8210312: JavaDoc example in SubmissionPublisher will potentially crash.

@openjdk
Copy link

openjdk bot commented Jun 18, 2025

@DougLea
Adding additional issue to issue list: 8292365: CompletableFuture and CompletionStage should document Memory Model guarantees.

@openjdk
Copy link

openjdk bot commented Jun 18, 2025

@DougLea
Adding additional issue to issue list: 8356304: Define "enabled" in ScheduledExecutorService.

@openjdk
Copy link

openjdk bot commented Jun 18, 2025

@DougLea 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
Copy link

mlbridge bot commented Jun 18, 2025

@liach
Copy link
Member

liach commented Jun 18, 2025

I think the problem in AtomicReferenceFieldUpdater being passed a static field exists for the other kinds of field updaters too. Currently the IAE is thrown by Unsafe, which is a hidden contract and can be inadvertently changed and affect the APIs; we might consider throwing IAE for static fields explicitly instead to avoid this pitfall.

* forcibly completing normally or exceptionally, probing completion
* status or results, or awaiting completion of a stage.
* Implementations of CompletionStage may provide means of achieving
* such effects, as appropriate. Method {@link #toCompletableFuture}
* enables interoperability among different implementations of this
* interface by providing a common conversion type.
*
* <p>Memory consistency effects: Actions in a thread prior to the
* submission of a computation producing a {@code CompletionStage}
* <i>happen-before</I> that computation begins. And actions taken by
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* <i>happen-before</I> that computation begins. And actions taken by
* <i>happen-before</i> that computation begins. And actions taken by

* submission of a computation producing a {@code CompletionStage}
* <i>happen-before</I> that computation begins. And actions taken by
* a {@code CompletionStage} <i>happen-before</i> actions of any
* dependent stage subsequent to its completion.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found "its" ambiguous in that sentence, does it mean:

Suggested change
* dependent stage subsequent to its completion.
* dependent stage subsequent to that stage's completion.

🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to:
And actions taken by

  • {@code CompletionStage x} happen-before actions of any
  • dependent stage subsequent to {@code x}'s completion.

* submit(function.apply(item));
* subscription.request(1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -46,7 +46,7 @@

/**
* A reflection-based utility that enables atomic updates to
* designated {@code volatile} reference fields of designated
* designated non-static {@code volatile} reference fields of designated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't this change also apply to the other AtomicXFieldUpdaters?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Will do. Also, as mentioned by @liach we could trap violations by throwing a better exception. But considering that people should be using VarHandles instead anyway these days, and old uses might depend on current behavior, it doesn't seem worthwhile to add? (line 338)

  •        if (Modifier.isStatic(modifiers))
    
  •            throw new IllegalArgumentException("Must not be static");
    

@DougLea
Copy link
Contributor Author

DougLea commented Jun 19, 2025

/issue JDK-8353155

@openjdk
Copy link

openjdk bot commented Jun 19, 2025

@DougLea
Adding additional issue to issue list: 8353155: FutureTask#run(): doc implies synchronous, implementation is async.

@DougLea
Copy link
Contributor Author

DougLea commented Jun 19, 2025

/issue JDK-8186959

@openjdk
Copy link

openjdk bot commented Jun 19, 2025

@DougLea
Adding additional issue to issue list: 8186959: Clarify that Executors.newScheduledThreadPool() is fixed-size.

@DougLea
Copy link
Contributor Author

DougLea commented Jun 19, 2025

/issue JDK-8190889

@openjdk
Copy link

openjdk bot commented Jun 19, 2025

@DougLea
Adding additional issue to issue list: 8190889: TimeUnit.wait should document IllegalMonitorStateException.

@DougLea
Copy link
Contributor Author

DougLea commented Jun 19, 2025

/issue JDK-6351533

@DougLea
Copy link
Contributor Author

DougLea commented Jun 19, 2025

/issue JDK-6317534

@openjdk
Copy link

openjdk bot commented Jun 19, 2025

@DougLea
Adding additional issue to issue list: 6351533: CyclicBarrier reset() should return the number of awaiters.

@openjdk
Copy link

openjdk bot commented Jun 19, 2025

@DougLea
Adding additional issue to issue list: 6317534: CyclicBarrier should have a cancel() method.

@DougLea
Copy link
Contributor Author

DougLea commented Jun 19, 2025

/issue JDK-8195628

@openjdk
Copy link

openjdk bot commented Jun 19, 2025

@DougLea
Adding additional issue to issue list: 8195628: Documentation for lock(), trylock(), lockInterruptibly​() of ReentrantReadWriteLock.WriteLock needs to be corrected.

@DougLea
Copy link
Contributor Author

DougLea commented Jun 19, 2025

/issue JDK-8333172

@openjdk
Copy link

openjdk bot commented Jun 19, 2025

@DougLea
Adding additional issue to issue list: 8333172: Document a recommendation to use VarHandles instead of java.util.concurrent.atomic.*FieldUpdater.

@@ -398,6 +398,8 @@ else if (s == MICRO_SCALE)
* @param obj the object to wait on
* @param timeout the maximum time to wait. If less than
* or equal to zero, do not wait at all.
* @throws IllegalMonitorStateException if the current thread is not
* the owner of this object's monitor.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably use "the object's monitor" rather than "the object's monitor" as "this" is the TimeUnit instance.

* return LEFT.compareAndSet(this, expect, update);
* }
* // ... and so on
* }}</pre>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this should be convert to a {@snippet ..} and have it handle the checked exceptions thrown by findVarHandle. I'm just thinking of someone seeing VarHandle usage for first time, then reporting a bug that the example doesn't compile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core-libs [email protected] rfr Pull request is ready for review
Development

Successfully merging this pull request may close these issues.

5 participants