On Fri, 21 May 2021 20:16:41 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: > > Second version of the fix test/jdk/java/awt/print/PrinterJob/ExceptionFromPrintableIsIgnoredTest.java line 32: > 30: @run main/manual ExceptionFromPrintableIsIgnoredTest MAIN RE > 31: @run main/manual ExceptionFromPrintableIsIgnoredTest EDT PE > 32: @run main/manual ExceptionFromPrintableIsIgnoredTest EDT RE wjy is this still manual ? test/jdk/java/awt/print/PrinterJob/ExceptionFromPrintableIsIgnoredTest.java line 85: > 83: PrinterJob job = PrinterJob.getPrinterJob(); > 84: if (job.getPrintService() == null) { > 85: throw new RuntimeException("No printers are available."); where does this exception go if it happens and you are on EDT ? test/jdk/java/awt/print/PrinterJob/ExceptionFromPrintableIsIgnoredTest.java line 120: > 118: (exceptionType == TestExceptionType.PE)) || > 119: ((printEx instanceof RuntimeException) && > 120: (exceptionType == TestExceptionType.RE))) { 1) I am fairly sure that I tested this case on Windows and nothing caught it so does this test work on Windows ? 2) I raised the question / suggestion that should we not wrap these in a PrinterException ? It at least needs to be thought through. test/jdk/java/awt/print/PrinterJob/ExceptionFromPrintableIsIgnoredTest.java line 124: > 122: } > 123: throw new RuntimeException("Unexpected exception was > thrown."); > 124: } else { When running on the EDT do you really want to do this ? ------------- PR: https://git.openjdk.java.net/jdk/pull/4036