On Wed, 19 May 2021 21:40:49 GMT, Phil Race <p...@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
>
> src/java.desktop/macosx/classes/sun/lwawt/macosx/CPrinterJob.java line 823:
> 
>> 821:                         }
>> 822:                     } catch (Exception e) {
>> 823:                         // Original code bailed on any exception
> 
> Throwable ?
> And what was wrong with bailing ? That comment has been there since the code 
> was brought into OPenJDK so I can't tell what the motivation was.
> But I'm getting a bit lost what is going on here. 
> 
> The spec 
> https://docs.oracle.com/en/java/javase/11/docs/api/java.desktop/java/awt/print/Printable.html#print(java.awt.Graphics,java.awt.print.PageFormat,int)
> 
> says 
> Throws:
> PrinterException - thrown when the print job is terminated.
> 
> I take that to mean we should not carry on printing .. but it looks like we 
> just note the exception and carry on.
> I'm not sure this code (pre-existing behaviour) is right.
> 
> We do want to grab the exception so it doesn't mess up interverning code but 
> don't we want this to reach the caller of PrinterJob.print - in the form of a 
> PrinterExcepton ?

In the 2nd version of the fix I substituted "Exception" for "Throwable" in this 
catch block and in second "catch" block several lines below. Also in the 2nd 
fix version I removed this comment about bailing.

My opinion is the same, the code before the fix did not work according to the 
specification and was ignoring any caught exceptions. Perhaps, the reason of 
this is involvement of more than 1 thread in the code flow standing behind 
"PrinterJob.print" method execution and execution of the JDK's code through JNI 
in this code flow. Thus propagating the exception is impossible across threads 
and it is not easy to stop printing done by macOS at the moment, when the 
exception occurs in Java code in JDK. This pattern is used in the file 
"CPrinterJob.java" in other places, even there is the same comment in the 
method "CPrinterJob.getPageformatPrintablePeekgraphics". At the same time 
propagating "RuntimeException" from "PrinterJob.print" on Windows, Linux and 
now on macOS after this fix also is not according to the specification but 
still consistency in JDK behavior on these OS versions should be good.

As I understand, the method "CPrinterJob.printAndGetPageFormatArea" returns 
"null" as the "Rectangle2D", when the exception is caught, and this "null" 
value should automatically lead to returning of "NSZeroRect" from the 
Objective-C method "(NSRect)rectForPage:(NSInteger)pageNumber" in the file 
"src/java.desktop/macosx/native/libawt_lwawt/awt/PrinterView.m", from which 
"CPrinterJob.printAndGetPageFormatArea" Java method was invoked. And this 
"NSZeroRect" return value is indication of the error for macOS, which further 
leads to stopping of the printing process.

-------------

PR: https://git.openjdk.java.net/jdk/pull/4036

Reply via email to