[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-2d-dev
Subject: [OpenJDK 2D-Dev] Suggest a modification to isPostscript exception handling
From: philip.race () oracle ! com (Phil Race)
Date: 2012-08-27 17:57:33
Message-ID: 503BB50D.1020609 () oracle ! com
[Download RAW message or body]
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; 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/%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/%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>
> <mailto:zhouyx at linux.vnet.ibm.com
> <mailto: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>
> <mailto:2d-dev at openjdk.java.net
> <mailto: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/%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
>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic