[OpenJDK 2D-Dev] Suggest a modification to isPostscript exception handling
Sean Chou
zhouyx at linux.vnet.ibm.com
Mon Sep 3 02:34:48 UTC 2012
Hi Phil,
Yes, a non-IO Exception is generated. Another catch clause is of
cause a choice;
however we are thinking the return true might wide the exception too much,
so it is
modified.
On Tue, Aug 28, 2012 at 1:57 AM, Phil Race <philip.race at oracle.com> wrote:
> Hi,
>
>
> >The reason is quite straight forward as described in the first mail.
> >IPPPrintService.isPostscript catches all IOException and return true,
> > while the try block in PSPrinterJob catches all Throwable .
> >In our scenario, an non-io exception is produced in the try block and it
> get executed.
>
> So you mean the code in IPPPrintService somehow generates a non-IO
> Exception?
> In that case shouldn't you widen the catch there rather in the outer code ?
> What precise exception happened, precisely where and precisely why ?
>
> As for the bug you point to, the Lexmark T632 is a Postscript capable
> printer
> so if it had problems with valid postscript, then its a Lexmark bug. I
> don't
> think we should bias the implementation to work around a bug in a
> particular printer.
>
> Also the source code of IPPPrintService has a comment which already
> answered your
> question as to why we chose to return true - it was expecting an
> IOException to most likely
> mean no PPD meaning a raw printer. Since we are sending postscript it had
> then better be a
> postscript capable printer ..
> Other reasons probably were indicative of being unable to print there at
> all.
>
> -phil.
>
>
>
> On 8/26/2012 5:24 AM, Sean Chou wrote:
>
>> Hi Phil,
>>
>>
>> On Sat, Aug 25, 2012 at 2:10 AM, Phil Race <philip.race at oracle.com<mailto:
>> philip.race at oracle.com**>> wrote:
>>
>> Hi,
>>
>> On 8/23/2012 10:23 PM, Sean Chou wrote:
>>
>> Hi Phil,
>>
>> I'm really sorry about this typo, the modification looks
>> so simple that I became careless when porting.
>>
>>
>> That's a slippery slope. Only send out code that you built and tested.
>> What if reviewer also thinks "this must be OK else it wouldn't
>> have built, and of course he built it ... "
>>
>> You are right, that's my fault and it won't happen again.
>>
>>
>>
>> The patch is from ibmjdk and it has been tested on Java6
>> since Oct, 2007 and on Java7 since Feb, 2012. To be honest,
>> this modification isn't related to a real bug in openjdk for
>> now; it is posted to see if there are any reason that the
>> printer is assumed to be a postscript printer in all exceptions.
>>
>>
>> The code went into JDK 6u2 on 27th Feb 2007 and would have been
>> released a few months later.
>> IBM apparently made this change pretty soon after that.
>> So the question that should be asked is not why is the openjdk
>> code like this,
>> but why did IBM make the change they did ?
>> I have no record or recollection of any problem reports.
>> I can't see any value to it. Unless there's a reflection problem
>> (which there should not be!)
>> its never going to get executed.
>>
>>
>> The reason is quite straight forward as described in the first mail.
>> IPPPrintService.isPostscript catches all IOException and return true, while
>> the try block in PSPrinterJob catches all Throwable . In our scenario, an
>> non-io exception is produced in the try block and it get executed.
>>
>> As there are some bugs related to "/DeferredMediaSelection true" like
>> http://bugs.sun.com/**bugdatabase/view_bug.do?bug_**id=6266343<http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6266343>;
>> and it is strange to return true when exceptions are seen because the
>> default behavior in try block is to return true. We think the expansion
>> from IOException to Throwable is too much, and return false for exception
>> is better. Does it return true because it is assumed not to run ?
>>
>>
>> -phil.
>>
>>
>> Many thanks.
>>
>> The false is corrected in this webrev:
>> http://cr.openjdk.java.net/~**zhouyx/OJDK-429/webrev.02/<http://cr.openjdk.java.net/~zhouyx/OJDK-429/webrev.02/>
>> <http://cr.openjdk.java.net/%**7Ezhouyx/OJDK-429/webrev.02/<http://cr.openjdk.java.net/%7Ezhouyx/OJDK-429/webrev.02/>
>> >
>> <http://cr.openjdk.java.net/%**7Ezhouyx/OJDK-429/webrev.02/<http://cr.openjdk.java.net/%7Ezhouyx/OJDK-429/webrev.02/>
>> >
>>
>>
>> On Fri, Aug 24, 2012 at 12:20 AM, Phil Race
>> <philip.race at oracle.com <mailto:philip.race at oracle.com**>
>> <mailto:philip.race at oracle.com
>>
>> <mailto:philip.race at oracle.com**>>> wrote:
>>
>> Sean,
>>
>> Without even commenting on the merits or necessity I note
>> that you
>> cannot possibly
>> have even built this patch, much less tested it.
>>
>> > 625 return Boolean.FLASE;
>>
>> -phil.
>>
>>
>> On 8/23/12 1:58 AM, Sean Chou wrote:
>>
>> Hello,
>>
>> I updated the repository to 2d, the webrev is now:
>> http://cr.openjdk.java.net/~**zhouyx/OJDK-429/webrev.01/<http://cr.openjdk.java.net/~zhouyx/OJDK-429/webrev.01/>
>> <http://cr.openjdk.java.net/%**7Ezhouyx/OJDK-429/webrev.01/<http://cr.openjdk.java.net/%7Ezhouyx/OJDK-429/webrev.01/>
>> >
>> <http://cr.openjdk.java.net/%**7Ezhouyx/OJDK-429/webrev.01/<http://cr.openjdk.java.net/%7Ezhouyx/OJDK-429/webrev.01/>
>> >
>>
>> Please take a look.
>>
>> ---------- Forwarded message ----------
>> From: *Sean Chou* <zhouyx at linux.vnet.ibm.com
>> <mailto:zhouyx at linux.vnet.ibm.**com<zhouyx at linux.vnet.ibm.com>
>> >
>> <mailto:zhouyx at linux.vnet.ibm.**com<zhouyx at linux.vnet.ibm.com>
>> <mailto:zhouyx at linux.vnet.ibm.**com<zhouyx at linux.vnet.ibm.com>
>> >>>
>> Date: Thu, Aug 23, 2012 at 2:24 PM
>> Subject: Suggest a modification to isPostscript
>> exception handling
>> To: 2d-dev at openjdk.java.net
>> <mailto:2d-dev at openjdk.java.**net <2d-dev at openjdk.java.net>>
>> <mailto:2d-dev at openjdk.java.**net <2d-dev at openjdk.java.net>
>>
>> <mailto:2d-dev at openjdk.java.**net <2d-dev at openjdk.java.net>>>
>>
>>
>> Hello,
>>
>> This is a simple modification to
>> sun/print/PSPrinterJob.java.
>> When sun.print.IPPPrintService.**isPostscript
>> method checks if
>> the printer is a postscript printer, if IOException
>> happens, the
>> method assumes the printer is postscript printer
>> (IPPPrintService.java, line 1605). In class
>> PSPrinterJob, it
>> invoke isPostscript and assumes all Throwables to be a
>> postscript printer ( PSPrinterJob.java, line 625 ). I
>> think it
>> should return false in cases exceptions other
>> than IOException are caught, IOException should not be
>> expanded
>> to all Throwable.
>>
>> The webrev is at:
>> http://cr.openjdk.java.net/~**zhouyx/OJDK-429/webrev.00/<http://cr.openjdk.java.net/~zhouyx/OJDK-429/webrev.00/>
>> <http://cr.openjdk.java.net/%**7Ezhouyx/OJDK-429/webrev.00/<http://cr.openjdk.java.net/%7Ezhouyx/OJDK-429/webrev.00/>
>> >
>> <http://cr.openjdk.java.net/%**7Ezhouyx/OJDK-429/webrev.00/<http://cr.openjdk.java.net/%7Ezhouyx/OJDK-429/webrev.00/>>
>> .
>>
>>
>> Please take a look.
>>
>> -- Best Regards,
>> Sean Chou
>>
>>
>>
>>
>>
>> -- Best Regards,
>> Sean Chou
>>
>>
>>
>>
>>
>> --
>> Best Regards,
>> Sean Chou
>>
>>
>
--
Best Regards,
Sean Chou
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20120903/d5b0f24a/attachment.html>
More information about the 2d-dev
mailing list