TIBCOSoftware / genxdm

GenXDM: XQuery/XPath Data Model API, bridges, and processors for tree-model neutral access to XML.
http://www.genxdm.org/
9 stars 4 forks source link

review try/finally usage ; when we can require jse 7, switch to arm blocks #163

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
Our codebase contains a number of try/finally blocks with unusual semantics. 
Rather than handling resource leaks, these blocks attempt to maintain state, 
and in some cases even attempt to fix errors.

This is bad mostly because many of the finally blocks aren't bullet-proofed, 
and attempts to change state inside these blocks may throw exceptions, which 
may then be masking a serious exception. A classic case of this was the use of 
finally blocks when writing elements to ensure that an element close tag was 
always written once the element start tag had been. Which produced the most 
insane XML output imaginable, as it produced structurally (but not namespace) 
well-formed output, with completely obscure exceptions that obviously were 
unrelated to the (highly visible) corruption. In other examples, we're managing 
stacks with a conditional .pop() in a finally block, when the try may produce 
an assertion error. An empty stack generates an all-new, trivial exception that 
causes the assertion to be discarded. Fun times.

We need to find all of these blocks and determine whether we should be doing 
these things; if we continue to use try/finally, we must ensure that we handle 
even runtime exceptions that could handle for state modification methods called 
inside them. By preference, we would move to requiring jse 7, and aggressively 
replace these try/finally blocks with try-with-resource blocks using resources 
that implement autocloseable, and then we would refactor the remaining 
try/finally blocks that do state management to avoid possible masking (ideally 
by doing our state management without hail-mary blocks).

Original issue reported on code.google.com by aale...@gmail.com on 8 Jul 2015 at 2:49