8345144: Robot does not specify all causes of IllegalThreadStateException #22564

alisenchung · 2024-12-04T22:33:41Z

When robot.autoWaitForIdle is set to true, all mouse and key-related methods when invoked on the EDT will throw java.lang.IllegalThreadStateException which is not in the Robot specification.

This PR updates the specification by adding warnings to avoid calling lengthy and delay-type methods on EDT and including exceptions thrown when autoWaitForIdle is set to true and mouse/key-handling methods are called on the EDT.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Change requires CSR request JDK-8346687 to be approved
Commit message must refer to an issue

Issues

JDK-8345144: Robot does not specify all causes of IllegalThreadStateException (Bug - P4)
JDK-8346687: Robot does not specify all causes of IllegalThreadStateException (CSR)

Reviewers

Phil Race (@prrace - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 22564

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

Using diff file

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

Using Webrev

Link to Webrev Comment

bridgekeeper · 2024-12-04T22:34:14Z

👋 Welcome back achung! 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 · 2024-12-04T22:34:36Z

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

8345144: Robot does not specify all causes of  IllegalThreadStateException

Reviewed-by: prr

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

27646e5: 8344148: Add an explicit compiler phase for warning generation
8d388cc: 8346457: AOT cache creation crashes with "assert(pair_at(i).match() < pair_at(i+1).match()) failed: unsorted table entries"
d723597: 8345432: (ch, fs) Replace anonymous Thread with InnocuousThread
9c393a2: 8346953: Remove unnecessary @SuppressWarnings annotations (client, Make "can write" check work on Windows #2)
9a60f44: 8345668: ZoneOffset.ofTotalSeconds performance regression
12700cb: 8346264: "Total compile time" counter should include time spent in failing/bailout compiles
dd81f8d: 8344079: Minor fixes and cleanups to compiler lint-related code
ccf3d57: 8346985: Convert test/jdk/com/sun/jdi/ClassUnloadEventTest.java to Class-File API
594e519: 8346984: Remove ASM-based benchmarks from Class-File API benchmarks
c027f2e: 8346983: Remove ASM-based transforms from Class-File API tests
... and 261 more: https://git.openjdk.org/jdk/compare/f904480a49649e0e89e4afca11fcadecb8669331...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 · 2024-12-04T22:35:26Z

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

client

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 · 2024-12-04T22:37:50Z

Webrevs

kumarabhi006 · 2024-12-05T04:21:54Z

Needs a CSR ?

honkar-jdk

Doc changes look good to me. As @kumarabhi006 pointed, a corresponding CSR is required.

I have the same question as @azvegint suggested earlier: In case of robot.delay() is it good to have a recommendation - "to not call on EDT" or would it be better if the check is done by the method itself to avoid relying on the end-user to check it?

public void delay(int ms) {
    if (isEventDispatchThread()) {
        throw new IllegalThreadStateException("delay() called on EDT");
    }

azvegint · 2024-12-10T13:32:44Z

/csr needed

openjdk · 2024-12-10T13:32:59Z

@azvegint has indicated that a compatibility and specification (CSR) request is needed for this pull request.

@alisenchung please create a CSR request for issue JDK-8345144 with the correct fix version. This pull request cannot be integrated until the CSR request is approved.

azvegint · 2024-12-10T13:38:10Z

public void delay(int ms) {
    if (isEventDispatchThread()) {
        throw new IllegalThreadStateException("delay() called on EDT");
    }

There is already a checkNotDispatchThread method, so we can reuse it.

And please do a build with your changes as it currently fails:

src/java.desktop/share/classes/java/awt/Robot.java:671: warning: empty <p> tag
     * <p>
       ^
error: warnings found and -Werror specified

https://github.com/alisenchung/jdk/actions/runs/12169487260/job/33942544112

prrace · 2024-12-18T22:29:42Z

src/java.desktop/share/classes/java/awt/Robot.java

@@ -58,6 +58,14 @@
 * queue. For example, {@code Robot.mouseMove} will actually move
 * the mouse cursor instead of just generating mouse move events.
 * <p>
+ * Note: {@code waitForIdle()} must not be called on the AWT EDT, since


The word "and" needs to be before "since" for this to make sense and then that needs more rewording later.

Mulling it over, I suggest rewording like this

* Note: When {@code autoWaitForIdle()} is enabled, mouse and key related methods * cannot be called on the AWT EDT. * This is because when {@code autoWaitForIdle()} is enabled, * the mouse and key methods implicitly call {@code waitForIdle()} * which will throw {@code IllegalThreadStateException} when called on the AWT EDT.

alisenchung · 2025-01-09T20:46:50Z

/integrate

openjdk · 2025-01-09T20:47:26Z

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

9f3c5f9: 8344907: NullPointerException in Win32ShellFolder2.getSystemIcon when "icon" is null
22f70a7: 8321413: IllegalArgumentException: Code length outside the allowed range while creating a jlink image
3bfa952: 8283795: Add TLSv1.3 and CNSA 1.0 algorithms to implementation requirements
1ef77cd: 8347171: (dc) java/nio/channels/DatagramChannel/InterruptibleOrNot.java fails with virtual thread factory
72f1114: 8346705: SNI not sent with Java 22+ using java.net.http.HttpClient.Builder#sslParameters
cb9a98b: 8347141: Several javac tests compile with an unnecessary -Xlint:-path flag
f9b1133: 8346880: [aix] java/lang/ProcessHandle/InfoTest.java still fails: "reported cputime less than expected"
9c72ded: 8346036: Unnecessary Hashtable usage in javax.swing.text.html.parser.Entity
3024a73: 8345782: Refining the cases that libjsig deprecation warning is issued
2801bc6: 8346460: NotifyFramePop should return JVMTI_ERROR_DUPLICATE
... and 325 more: https://git.openjdk.org/jdk/compare/f904480a49649e0e89e4afca11fcadecb8669331...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-01-09T20:47:36Z

@alisenchung Pushed as commit df28cec.

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

init commit

c5e72b8

openjdk bot changed the title ~~8345144: Robot does not specify all causes of IllegalThreadStateException~~ 8345144: Robot does not specify all causes of IllegalThreadStateException Dec 4, 2024

openjdk bot added the rfr Pull request is ready for review label Dec 4, 2024

openjdk bot added the client [email protected] label Dec 4, 2024

honkar-jdk reviewed Dec 9, 2024

View reviewed changes

openjdk bot added the csr Pull request needs approved CSR before integration label Dec 10, 2024

alisenchung added 3 commits December 11, 2024 11:04

remove extra html tag, update wording

9378c16

remove extra space

f83ec89

align spacing

4928e4b

prrace reviewed Dec 18, 2024

View reviewed changes

alisenchung added 2 commits December 19, 2024 11:47

update class spec

89a48d3

api note

164e927

prrace approved these changes Dec 19, 2024

View reviewed changes

openjdk bot added ready Pull request is ready to be integrated and removed csr Pull request needs approved CSR before integration labels Jan 6, 2025

openjdk bot added the integrated Pull request has been integrated label Jan 9, 2025

openjdk bot closed this Jan 9, 2025

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Jan 9, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8345144: Robot does not specify all causes of IllegalThreadStateException #22564

8345144: Robot does not specify all causes of IllegalThreadStateException #22564

alisenchung commented Dec 4, 2024 •

edited by openjdk bot

Loading

bridgekeeper bot commented Dec 4, 2024

openjdk bot commented Dec 4, 2024 •

edited

Loading

openjdk bot commented Dec 4, 2024

mlbridge bot commented Dec 4, 2024 •

edited

Loading

kumarabhi006 commented Dec 5, 2024

honkar-jdk left a comment

azvegint commented Dec 10, 2024

openjdk bot commented Dec 10, 2024

azvegint commented Dec 10, 2024 •

edited

Loading

prrace Dec 18, 2024

alisenchung commented Jan 9, 2025

openjdk bot commented Jan 9, 2025

openjdk bot commented Jan 9, 2025

8345144: Robot does not specify all causes of IllegalThreadStateException #22564

8345144: Robot does not specify all causes of IllegalThreadStateException #22564

Conversation

alisenchung commented Dec 4, 2024 • edited by openjdk bot Loading

Progress

Issues

Reviewers

Reviewing

bridgekeeper bot commented Dec 4, 2024

openjdk bot commented Dec 4, 2024 • edited Loading

openjdk bot commented Dec 4, 2024

mlbridge bot commented Dec 4, 2024 • edited Loading

Webrevs

kumarabhi006 commented Dec 5, 2024

honkar-jdk left a comment

Choose a reason for hiding this comment

azvegint commented Dec 10, 2024

openjdk bot commented Dec 10, 2024

azvegint commented Dec 10, 2024 • edited Loading

prrace Dec 18, 2024

Choose a reason for hiding this comment

alisenchung commented Jan 9, 2025

openjdk bot commented Jan 9, 2025

openjdk bot commented Jan 9, 2025

alisenchung commented Dec 4, 2024 •

edited by openjdk bot

Loading

openjdk bot commented Dec 4, 2024 •

edited

Loading

mlbridge bot commented Dec 4, 2024 •

edited

Loading

azvegint commented Dec 10, 2024 •

edited

Loading