<AWT Dev> RFR: 8262731: [macOS] Exception from "Printable.print" is swallowed during "PrinterJob.print" [v2]
prr at openjdk.java.net
Tue Jun 8 20:06:16 UTC 2021
On Fri, 21 May 2021 20:16:41 GMT, Anton Litvinov <alitvinov at openjdk.org> wrote:
>> 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 Litvinov has updated the pull request incrementally with one additional commit since the last revision:
> Second version of the fix
The points I have been trying to make are that
1) We need to make sure that important cleanup happens rather than just bailing
2) Despite Windows + Linux propagating a RTE, I am not sure this is what we want.
If the RTE is something the *app* decides to throw, then that's one thing but
if something went wrong in the bowels of printing the spec says it should be a PrinterException.
At the very least we should make sure temp files are deleted, any open calls
to startDoc have been matched with a close ..
Hence my early comment "The exception should not propagate. State may need to be cleaned up.
If you do anything you would make it the init cause of the PrinterException."
That means if we see an RTE propagated .. that could be a bug.
So I am nor comfortable just "aligning" the behaviour with WIndows and Linux without being
sure they are correct.
In other words, mac was probably catching this to do the clean up part, and that was likely OK
But yes, it probably was wrong not to rethrow as a wrapped (if needed) PrinterException
But although it opens a whole new can of worms I was suggesting you needed to look closely
at windows + linux and not just create a test which verified behaviour I am not sure we want.
I questioned the test etc about RTE because although it is good to test hrowing RTE you
are verifying it ARRIVES still as an RTE, and I think it should be a PrinterException.
I am open to discussion about why this may be "actually OK" but it has to be better than
"the others are doing it" and it definitely should not be a reason to leave open files/jobs.
More information about the awt-dev