Request for review: More cause-chaining for InternalError

[Alan Bateman]

Sebastian Sickelmann wrote:
> Yes. It's more a assiduity work*, then real complex.
> The only non trival stuff maybe 
> ***src/share/classes/java/text/DecimalFormat.java* 
> <http://dict.leo.org/ende?lp=ende&p=Ci4HO3kMAA&search=assiduity&trestr=0x8001> 
> [0]
> in combination with src/share/classes/java/text/Format.java [1]
>
> Is there someone who whats to sponsor this by creating a CR and 
> pushing it? Or are there more reviewers needed?
>
> How is such cleanup-work handled? I can make more assiduity work on 
> this, but is it worth it? Are there drawbacks, additional to my 
> mailing-list spam, by such cleanups? Chaining more aggresivly maybe 
> cause CG to do later cleanup and serialized Exceptions can be more 
> expensive while serializing and transfering over wire(remote-calls etc.).
>
I've created a bug for this:
7084245: Update usages of InternalError to use exception chaining

and Sherman is going to help get this pushed.

I scanned the patch and see that you've updated three of the 
java.util.concurrent classes. The usual protocol here is for the changes 
to go first into Doug Lea's upstream CVS and then we periodically pull 
the changes into OpenJDK. Chris Hegarty currently looks after this. 
Everyone involved is flexible and small changes like this aren't a major 
issue, it's really just to avoid the code diverging and also making it 
easy to merge/sync up regularly.

You've added a "TODO" to sun.java2d.pipe.LoopPipe.getStrokesSpans asking 
about the printStackTrace. Looks odd to me too but you could check on 
the 2d-dev list in case there is some reason it was put in there.

Kumar should be able to help with your question in sun.misc.Launcher.

I suspect future maintainers might be puzzled by the comment added to 
java.text.DecimalFormat.clone so I wonder if you should leave it out.

How to handle clean-up work in general is a good question, especially 
with patches that touch many areas, like this patch. My personal view is 
that it's polite to copy the mailing lists for all the areas changed so 
that the folks have at least a heads-up and a chance to comment if they 
wish. Less surprises when they are forced to re-base their in-progress 
patches for example. One reviewer for the patch is fine and there is no 
requirement that you get a reviewer from every single area.

On the exception chaining then I suspect you'll find lots of 
opportunities in the code. You probably do need to watch out for 
exceptions that have references to objects that ultimately reference the 
world but I hope they would be few. Otherwise I wouldn't be too 
concerned about making more use of exception chaining.

-Alan.