On Fri, 11 Jun 2021 17:06:24 GMT, Anton Litvinov <alitvi...@openjdk.org> wrote:
>> Hello,
>>
>> Could you please review the following fix for the bug specific to macOS. The
>> bug consists in the fact that if the method
>> "java.awt.print.Printable.print​(Graphics, PageFormat, int)" throws
>> "java.awt.print.PrinterException" or "java.lang.RuntimeException" during the
>> call "java.awt.print.PrinterJob.print()", then the exception is caught and
>> ignored by JDK and a user cannot learn that printing failed and what caused
>> failure of printing, because "PrinterJob.print()" method does not throw
>> "PrinterException" or the occurred exception is not reported by JDK through
>> the error stream.
>>
>> ROOT CAUSE OF THE BUG:
>> The root cause of the bug is the fact that in the method
>> "sun.lwawt.macosx.CPrinterJob.printAndGetPageFormatArea(final Printable,
>> final Graphics, final PageFormat, final int)" from the file
>> "src/java.desktop/macosx/classes/sun/lwawt/macosx/CPrinterJob.java" the
>> exception thrown during execution of the expression
>>
>> "int pageResult = printable.print(graphics, pageFormat, pageIndex);"
>>
>> is caught but is not returned to a developer by any mean or is not printed
>> out to the error stream.
>>
>> THE FIX:
>> The fix implements propagation of the occurred and caught exception to the
>> level of the user's code executing "PrinterJob.print()" method. Propagation
>> of the exception by storing it in the instance variable of "CPrinterJob"
>> object is implemented, because the engaged code always is executed:
>> - on 2 threads (non-EDT thread, EDT thread) in case when
>> "PrinterJob.print()" is called by the user on a non-EDT thread;
>> - on 3 threads (2 EDT threads, a temporary thread started by JDK to execute
>> "CPrinterJob._safePrintLoop(long, long );") when "PrinterJob.print()" is
>> called on EDT thread.
>>
>> The regression test which is part of the fix was also successfully executed
>> on MS Windows OS and Linux OS.
>>
>> Thank you,
>> Anton
>
> Anton Litvinov has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Third version of the fix
Hello Phil,
I created the 3rd version of the fix which should address your remarks about
the fix and the regression test. Can you please review the 3rd version of the
fix (Webrev 03).
Thank you for provision with your position about different aspects of the fix,
regression test and the discovered discrepancies with the specification on
Windows OS and Linux OS. I absolutely agree that it is incorrect to implement
propagation of RTE through "PrinterJob.print()" without wrapping it into
"PrinterException", because RTE is not mentioned in the specification for
"PrinterJob.print()" method. The 3rd fix version addresses this mistake from
previous fix versions.
Perhaps, it is not very significant issue, that on Windows OS and Linux OS RTE
from "Printable.print​(Graphics, PageFormat, int)" is propagated, because in
this case the user's code does not follow the specification by throwing RTE
from "Printable.print​(Graphics, PageFormat, int)". I do not want to
investigate this discovered discrepancy with specification on Windows OS and
Linux OS in this fix for a little different issue specific to macOS. If it is
currently really necessary I can file a bug or 2 bugs in JBS to address the
discovered issue on Windows and Linux.
Thank you for giving me the details about "printer" value for "@key" tag in
"jtreg" test, because until that moment I mistakenly thought that it was just a
marker used for selection of tests marked with it for running and nothing else.
DIFFERENCES INTRODUCED IN THE 3'RD VERSION OF THE FIX:
THE FIX:
1. "RuntimeException" is wrapped in new "PrinterException" in
"CPrinterJob.print(PrintRequestAttributeSet)" to guarantee that the fix can
propagate only "PrinterException" from "PrinterJob.print()".
THE REGRESSION TEST:
1. The test is now fully automatic, "manual" is removed from all JTREG "main"
actions. I verified it on MACH and locally.
2. I addressed your concerns from previous comments and now no exception is
thrown from "ExceptionFromPrintableIsIgnoredTest.runTest" method, so there is
no any possibility that AWT thread will be crashed by the exception thrown by
the test. Now I just execute "PrinterJob.print()" and store any "Throwable"
possibly thrown by that method in the instance variable "private volatile
Throwable printError" of the test class. And analyze it always on main thread
after completion of execution of "ExceptionFromPrintableIsIgnoredTest.runTest"
call.
3. Now the regression test always expects only "PrinterException" to be thrown
by "PrinterJob.print()". Since "RuntimeException" is propagated from
"PrinterJob.print()" on Windows OS and on Linux OS two test scenarios with
"RuntimeException":
- "@run main ExceptionFromPrintableIsIgnoredTest MAIN RE"
- "@run main ExceptionFromPrintableIsIgnoredTest EDT RE"
are not executed by the test on Windows and Linux deliberately, otherwise they
are failing. This test restriction is implemented by the "if" condition "if
((exceptionType == TestExceptionType.RE) && !isOSX) {".
Hopefully this version of the fix is more acceptable.
Thank you,
Anton
-------------
PR: https://git.openjdk.java.net/jdk/pull/4036
