-
Notifications
You must be signed in to change notification settings - Fork 465
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
8295324: JavaFX: Blank pages when printing #916
8295324: JavaFX: Blank pages when printing #916
Conversation
👋 Welcome back eduardsdv! A progress list of the required criteria for merging this PR into |
Webrevs
|
/reviewers 2 |
@kevinrushforth |
Please review |
It seems that not all errors were fixed with this PR. Yesterday, a blank page was printed again. I will analyze what this can be due to. |
The test now also checks for the second race condition around 'jobDone' flag, which is in the print(Graphics, PageFormat, int) method.
This occurs in print(Graphics, PageFormat, int) if the 'jobDone' flag was previously set by the 'endJob()' method.
I found the second place with the race condition. It was in the print(Graphics g, PageFormat pf, int pageIndex) method. Please review. |
@prrace Can you review this? |
Can someone else check this PR if @prrace is not available? |
I think this is OK, but whilst I can see you went to some lengths to create a test case, I do not think it justifies |
Ok. I have reverted all changes regarding refactoring and testing. |
@eduardsdv 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:
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 43 new commits pushed to the
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. As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@prrace, @kevinrushforth) but any other Committer may sponsor as well. ➡️ To flag this PR as ready for integration with the above commit message, type |
/integrate |
@eduardsdv |
/sponsor |
Going to push as commit 7b3c88b.
Your commit was automatically rebased without conflicts. |
@kevinrushforth @eduardsdv Pushed as commit 7b3c88b. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
This fixes a race condition between application and 'Print Job Thread' threads when printing.
The race condition occurs when application thread calls
endJob()
, which in effect sets thejobDone
flag to true,and when the 'Print Job Thread' thread was in the
synchronized
block inwaitForNextPage()
at that time.The 'Print Job Thread' thread checks
jobDone
flag after exiting thesynchronized
block and, if it is true, skips the last page.In this fix, not only the
jobDone
is checked, but also that there is no other page to be printed.It was also needed to introduce a new flag 'jobCanceled', to skip the page if the printing was canceled by 'cancelJob()'.
Progress
Issue
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx pull/916/head:pull/916
$ git checkout pull/916
Update a local copy of the PR:
$ git checkout pull/916
$ git pull https://git.openjdk.org/jfx pull/916/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 916
View PR using the GUI difftool:
$ git pr show -t 916
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/916.diff