JDK-7020047 : Project Coin: generate null-check around try-with-resources close call
  • Type: Bug
  • Component: specification
  • Sub-Component: language
  • Affected Version: 7
  • Priority: P3
  • Status: Closed
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2011-02-16
  • Updated: 2018-01-17
  • Resolved: 2011-03-02
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 7
7 b132Fixed
Related Reports
Relates :  
JDK-7020499 - Project Coin: improvements to try-with-resources desugaring
Relates :  
JDK-8194978 - Javac produces dead code for try-with-resource
Relates :  
JDK-7031273 - Project Coin: Sync with JSR 334 PR
Relates :  
JDK-7014734 - Project Coin: Allow optional trailing semicolon to terminate resources list in try-with-resources
Relates :  
JDK-6971877 - Project Coin: improve semantics of suppressed exceptions in try-with-resources
Description
After due consideration, the JSR 334 expert group has decided to alter the null-handling semantics of the try-with-resources statement: the automatically generated calls to close will be protected by a null-check.

That is, try-with-resources will not call close on a resource if the resource is null.
Comments
SUGGESTED FIX # HG changeset patch # User darcy # Date 1298073320 28800 # Node ID 75e25df5087311cb40c2c6504b94da11d59fa439 # Parent 51e643f41a3ac12cf2ab6a6dd87fe97597b99345 7020047: Project Coin: generate null-check around try-with-resources close call Reviewed-by: jjg --- a/src/share/classes/com/sun/tools/javac/comp/Lower.java Fri Feb 18 16:17:44 2011 +0000 +++ b/src/share/classes/com/sun/tools/javac/comp/Lower.java Fri Feb 18 15:55:20 2011 -0800 @@ -1425,25 +1425,55 @@ public class Lower extends TreeTranslato } } - /** Optionally replace a try statement with an automatic resource - * management (ARM) block. + /** + * Optionally replace a try statement with the desugaring of a + * try-with-resources statement. The canonical desugaring of + * + * try ResourceSpecification + * Block + * + * is + * + * { + * final VariableModifiers_minus_final R #resource = Expression; + * Throwable #primaryException = null; + * + * try ResourceSpecificationtail + * Block + * catch (Throwable #t) { + * #primaryException = t; + * throw #t; + * } finally { + * if (#resource != null) { + * if (#primaryException != null) { + * try { + * #resource.close(); + * } catch(Throwable #suppressedException) { + * #primaryException.addSuppressed(#suppressedException); + * } + * } else { + * #resource.close(); + * } + * } + * } + * * @param tree The try statement to inspect. - * @return An ARM block, or the original try block if there are no - * resources to manage. - */ - JCTree makeArmTry(JCTry tree) { + * @return A a desugared try-with-resources tree, or the original + * try block if there are no resources to manage. + */ + JCTree makeTwrTry(JCTry tree) { make_at(tree.pos()); twrVars = twrVars.dup(); - JCBlock armBlock = makeArmBlock(tree.resources, tree.body, 0); + JCBlock twrBlock = makeTwrBlock(tree.resources, tree.body, 0); if (tree.catchers.isEmpty() && tree.finalizer == null) - result = translate(armBlock); + result = translate(twrBlock); else - result = translate(make.Try(armBlock, tree.catchers, tree.finalizer)); + result = translate(make.Try(twrBlock, tree.catchers, tree.finalizer)); twrVars = twrVars.leave(); return result; } - private JCBlock makeArmBlock(List<JCTree> resources, JCBlock block, int depth) { + private JCBlock makeTwrBlock(List<JCTree> resources, JCBlock block, int depth) { if (resources.isEmpty()) return block; @@ -1497,16 +1527,16 @@ public class Lower extends TreeTranslato int oldPos = make.pos; make.at(TreeInfo.endPos(block)); - JCBlock finallyClause = makeArmFinallyClause(primaryException, expr); + JCBlock finallyClause = makeTwrFinallyClause(primaryException, expr); make.at(oldPos); - JCTry outerTry = make.Try(makeArmBlock(resources.tail, block, depth + 1), + JCTry outerTry = make.Try(makeTwrBlock(resources.tail, block, depth + 1), List.<JCCatch>of(catchClause), finallyClause); stats.add(outerTry); return make.Block(0L, stats.toList()); } - private JCBlock makeArmFinallyClause(Symbol primaryException, JCExpression resource) { + private JCBlock makeTwrFinallyClause(Symbol primaryException, JCExpression resource) { // primaryException.addSuppressed(catchException); VarSymbol catchException = new VarSymbol(0, make.paramName(2), @@ -1525,20 +1555,28 @@ public class Lower extends TreeTranslato List<JCCatch> catchClauses = List.<JCCatch>of(make.Catch(catchExceptionDecl, catchBlock)); JCTry tryTree = make.Try(tryBlock, catchClauses, null); - // if (resource != null) resourceClose; - JCExpression nullCheck = makeBinary(JCTree.NE, - make.Ident(primaryException), - makeNull()); - JCIf closeIfStatement = make.If(nullCheck, + // if (primaryException != null) {try...} else resourceClose; + JCIf closeIfStatement = make.If(makeNonNullCheck(make.Ident(primaryException)), tryTree, makeResourceCloseInvocation(resource)); - return make.Block(0L, List.<JCStatement>of(closeIfStatement)); + + // if (#resource != null) { if (primaryException ... } + return make.Block(0L, + List.<JCStatement>of(make.If(makeNonNullCheck(resource), + closeIfStatement, + null))); } private JCStatement makeResourceCloseInvocation(JCExpression resource) { // create resource.close() method invocation - JCExpression resourceClose = makeCall(resource, names.close, List.<JCExpression>nil()); + JCExpression resourceClose = makeCall(resource, + names.close, + List.<JCExpression>nil()); return make.Exec(resourceClose); + } + + private JCExpression makeNonNullCheck(JCExpression expression) { + return makeBinary(JCTree.NE, expression, makeNull()); } /** Construct a tree that represents the outer instance @@ -3573,7 +3611,7 @@ public class Lower extends TreeTranslato if (tree.resources.isEmpty()) { super.visitTry(tree); } else { - result = makeArmTry(tree); + result = makeTwrTry(tree); } } --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/tools/javac/TryWithResources/TwrNullTests.java Fri Feb 18 15:55:20 2011 -0800 @@ -0,0 +1,72 @@ +/* + * Copyright (c) 2011, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @bug 7020047 + * @summary Test null handling of try-with-resources statement + */ + +public class TwrNullTests { + /* + * Each try-with-resources statement generates two calls to the + * close method for each resource: one for when there is a primary + * exception present and the second for when a primary exception + * is absent. The null handling of both cases needs to be + * checked. + */ + public static void main(String... args) { + testNormalCompletion(); + testNoSuppression(); + } + + /* + * Verify empty try-with-resources on a null resource completes + * normally; no NPE from the generated close call. + */ + private static void testNormalCompletion() { + try(AutoCloseable resource = null) { + return; // Nothing to see here, move along. + } catch (Exception e) { + throw new AssertionError("Should not be reached", e); + } + } + + /* + * Verify that a NPE on a null resource is <em>not</em> added as a + * suppressed exception to an exception from try block. + */ + private static void testNoSuppression() { + try(AutoCloseable resource = null) { + throw new java.io.IOException(); + } catch(java.io.IOException ioe) { + Throwable[] suppressed = ioe.getSuppressed(); + if (suppressed.length != 0) { + throw new AssertionError("Non-empty suppressed exceptions", + ioe); + } + } catch (Exception e) { + throw new AssertionError("Should not be reached", e); + } + } +}
18-02-2011
EVALUATION Yes.
16-02-2011