Skip to content
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

WIP: 8231372: Correctly terminate secondary event loop in JFXPanel.setScene() #16

Closed
wants to merge 1 commit into from

Conversation

@mruzicka
Copy link

mruzicka commented Oct 16, 2019

Secondary event loop introduced as a means of synchronization with the JavaFX Application thread in [1] never terminates as the SecondaryLoop.exit() call is not reached because the thread is blocked in the SecondaryLoop.enter() call.
This patch fixes the problem by submitting the UI work (including the call to the SecondaryLoop.exit() method) before entering the secondary loop.

[1] 7cf2dfa

Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed
@bridgekeeper bridgekeeper bot added the oca label Oct 16, 2019
@bridgekeeper
Copy link

bridgekeeper bot commented Oct 16, 2019

Hi mruzicka, welcome to this OpenJDK project and thanks for contributing!

We do not recognize you as Contributor and need to ensure you have signed the Oracle Contributor Agreement (OCA). If you have not signed the OCA, please follow the instructions. Please fill in your GitHub username in the "Username" field of the application. Once you have signed the OCA, please let us know by writing /signed in a comment in this pull request.

If you already are an OpenJDK Author, Committer or Reviewer, please click here to open a new issue so that we can record that fact. Please use "Add GitHub user mruzicka" as summary for the issue.

If you are contributing this work on behalf of your employer and your employer has signed the OCA, please let us know by writing /covered in a comment in this pull request.

@mruzicka mruzicka force-pushed the mruzicka:JFXPanel-fix branch 2 times, most recently from e75fb9a to 204c012 Oct 16, 2019
@mruzicka mruzicka changed the title JDK-8231372: Correctly terminate secondary event loop in JFXPanel.setScene() 8231372: Correctly terminate secondary event loop in JFXPanel.setScene() Oct 16, 2019
@mruzicka
Copy link
Author

mruzicka commented Oct 16, 2019

@kevinrushforth I believe you should be able to move forward with the review of this PR now as I've filed a bug report for the problem and signed the OCA as required. (As I did not know I needed to have my github account associated with the OCA, I did not include it in my details. I've asked Dalibor Topic from the Java SE PM team to help me have that fixed or get in touch with you to confirm my signing of the OCA.)

@kevinrushforth
Copy link
Member

kevinrushforth commented Oct 16, 2019

@mruzicka once Dalibor connects your OCA with your GitHub account, this review will be able to proceed.

Secondary event loop introduced as a means of synchronization with the JavaFX
Application thread in [1] never terminates as the SecondaryLoop.exit() call is
not reached because the thread is blocked in the SecondaryLoop.enter() call.
This patch fixes the problem by submitting the UI work (including the call
to the SecondaryLoop.exit() method) before entering the secondary loop.

[1] 7cf2dfa
@mruzicka mruzicka force-pushed the mruzicka:JFXPanel-fix branch from 204c012 to 9483ccd Oct 16, 2019
@bridgekeeper bridgekeeper bot removed the oca label Oct 17, 2019
@openjdk openjdk bot added the rfr label Oct 17, 2019
@mlbridge
Copy link

mlbridge bot commented Oct 17, 2019

Webrevs

@kevinrushforth
Copy link
Member

kevinrushforth commented Oct 21, 2019

Do you have a test program that shows the bug?

Also, can you take a look at whether you could turn the test case into an automated test?

@kevinrushforth
Copy link
Member

kevinrushforth commented Nov 13, 2019

@mruzicka - are you interested in moving this forward? If so, there is a pending request for a test case that needs to be answered before the review can proceed.

@kevinrushforth kevinrushforth changed the title 8231372: Correctly terminate secondary event loop in JFXPanel.setScene() WIP: 8231372: Correctly terminate secondary event loop in JFXPanel.setScene() Dec 3, 2019
@kevinrushforth
Copy link
Member

kevinrushforth commented Dec 3, 2019

Marking as WIP pending a test case to reproduce it. If you are still interested in pursuing this, please let us know.

@openjdk openjdk bot removed the rfr label Dec 3, 2019
@kevinrushforth kevinrushforth self-requested a review Dec 3, 2019
@kevinrushforth kevinrushforth removed their request for review Jan 3, 2020
@kevinrushforth
Copy link
Member

kevinrushforth commented Jan 3, 2020

Closing this PR pending a test case to reproduce it. It can be reopened if a test case is provided.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.