-
Notifications
You must be signed in to change notification settings - Fork 38.9k
use try-with-resources statement
#35046
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -63,16 +63,14 @@ public BridgeMethodResolver(Map declToBridge, ClassLoader classLoader) { | |
| Set bridges = (Set) entry.getValue(); | ||
| try { | ||
| InputStream is = classLoader.getResourceAsStream(owner.getName().replace('.', '/') + ".class"); | ||
| if (is == null) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same |
||
| return resolved; | ||
| } | ||
| try { | ||
| new ClassReader(is) | ||
| .accept(new BridgedFinder(bridges, resolved), | ||
| ClassReader.SKIP_FRAMES | ClassReader.SKIP_DEBUG); | ||
| } finally { | ||
| is.close(); | ||
| } | ||
| try (is) { | ||
| if (is == null) { | ||
| return resolved; | ||
| } | ||
| new ClassReader(is) | ||
| .accept(new BridgedFinder(bridges, resolved), | ||
| ClassReader.SKIP_FRAMES | ClassReader.SKIP_DEBUG); | ||
| } | ||
| } catch (IOException ignored) {} | ||
| } | ||
| return resolved; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -61,20 +61,16 @@ public Class loadClass(String name) throws ClassNotFoundException { | |
| name.replace('.','/') + ".class" | ||
| ); | ||
|
|
||
| if (is == null) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same |
||
| try (is) { | ||
| if (is == null) { | ||
|
|
||
| throw new ClassNotFoundException(name); | ||
| throw new ClassNotFoundException(name); | ||
|
|
||
| } | ||
| try { | ||
| } | ||
|
|
||
| r = new ClassReader(is); | ||
| r = new ClassReader(is); | ||
|
|
||
| } finally { | ||
|
|
||
| is.close(); | ||
|
|
||
| } | ||
| } | ||
| } catch (IOException e) { | ||
| throw new ClassNotFoundException(name + ":" + e.getMessage()); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -153,8 +153,7 @@ public ReadableByteChannel readableChannel() throws IOException { | |
| */ | ||
| @Override | ||
| public long contentLength() throws IOException { | ||
| InputStream is = getInputStream(); | ||
| try { | ||
| try (InputStream is = getInputStream()) { | ||
| long size = 0; | ||
| byte[] buf = new byte[256]; | ||
| int read; | ||
|
|
@@ -163,14 +162,6 @@ public long contentLength() throws IOException { | |
| } | ||
| return size; | ||
| } | ||
| finally { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please revert. The exception thrown while closing the stream is not caught/logged anymore. |
||
| try { | ||
| is.close(); | ||
| } | ||
| catch (IOException ex) { | ||
| debug(() -> "Could not close content-length InputStream for " + getDescription(), ex); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,31 +40,15 @@ abstract class AbstractEmbeddedDatabaseConfigurer implements EmbeddedDatabaseCon | |
|
|
||
| @Override | ||
| public void shutdown(DataSource dataSource, String databaseName) { | ||
| Connection con = null; | ||
| try { | ||
| con = dataSource.getConnection(); | ||
| try (Connection con = dataSource.getConnection()) { | ||
| if (con != null) { | ||
| try (Statement stmt = con.createStatement()) { | ||
| stmt.execute("SHUTDOWN"); | ||
| } | ||
| } | ||
| } | ||
| catch (SQLException ex) { | ||
| } catch (SQLException ex) { | ||
| logger.info("Could not shut down embedded database", ex); | ||
| } | ||
| finally { | ||
| if (con != null) { | ||
| try { | ||
| con.close(); | ||
| } | ||
| catch (SQLException ex) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same. Please revert. |
||
| logger.debug("Could not close JDBC Connection on shutdown", ex); | ||
| } | ||
| catch (Throwable ex) { | ||
| logger.debug("Unexpected exception on closing JDBC Connection", ex); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -253,8 +253,7 @@ public static void executeSqlScript(Connection connection, EncodedResource resou | |
| blockCommentEndDelimiter, statements); | ||
|
|
||
| int stmtNumber = 0; | ||
| Statement stmt = connection.createStatement(); | ||
| try { | ||
| try (Statement stmt = connection.createStatement()) { | ||
| for (String statement : statements) { | ||
| stmtNumber++; | ||
| try { | ||
|
|
@@ -270,27 +269,19 @@ public static void executeSqlScript(Connection connection, EncodedResource resou | |
| warningToLog = warningToLog.getNextWarning(); | ||
| } | ||
| } | ||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please revert this new line change. |
||
| catch (SQLException ex) { | ||
| } catch (SQLException ex) { | ||
| boolean dropStatement = StringUtils.startsWithIgnoreCase(statement.trim(), "drop"); | ||
| if (continueOnError || (dropStatement && ignoreFailedDrops)) { | ||
| if (logger.isDebugEnabled()) { | ||
| logger.debug(ScriptStatementFailedException.buildErrorMessage(statement, stmtNumber, resource), ex); | ||
| } | ||
| } | ||
| else { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please revert this new line change. |
||
| } else { | ||
| throw new ScriptStatementFailedException(statement, stmtNumber, resource, ex); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please revert. I think this will catch all exceptions, not just when trying to close. |
||
| finally { | ||
| try { | ||
| stmt.close(); | ||
| } | ||
| catch (Throwable ex) { | ||
| logger.trace("Could not close JDBC Statement", ex); | ||
| } | ||
| } catch (Throwable ex) { | ||
| logger.trace("Could not close JDBC Statement", ex); | ||
| } | ||
|
|
||
| long elapsedTime = System.currentTimeMillis() - startTime; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -316,21 +316,19 @@ protected boolean doReceiveAndExecute(Object invoker, @Nullable Session session, | |
| boolean exposeResource = (!transactional && isExposeListenerSession() && | ||
| !TransactionSynchronizationManager.hasResource(obtainConnectionFactory())); | ||
| Observation observation = createObservation(message).start(); | ||
| Observation.Scope scope = observation.openScope(); | ||
| if (logger.isDebugEnabled()) { | ||
| logger.debug("Received message of type [" + message.getClass() + "] from consumer [" + | ||
| consumerToUse + "] of " + (transactional ? "transactional " : "") + "session [" + | ||
| sessionToUse + "]"); | ||
| } | ||
| try { | ||
| try (Observation.Scope ignored = observation.openScope()) { | ||
Pankraz76 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if (logger.isDebugEnabled()) { | ||
| logger.debug("Received message of type [" + message.getClass() + "] from consumer [" + | ||
| consumerToUse + "] of " + (transactional ? "transactional " : "") + "session [" + | ||
| sessionToUse + "]"); | ||
| } | ||
| messageReceived(invoker, sessionToUse); | ||
| if (exposeResource) { | ||
| TransactionSynchronizationManager.bindResource( | ||
| obtainConnectionFactory(), new LocallyExposedJmsResourceHolder(sessionToUse)); | ||
| } | ||
| doExecuteListener(sessionToUse, message); | ||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please revert this new line change. |
||
| catch (Throwable ex) { | ||
| } catch (Throwable ex) { | ||
| if (status != null) { | ||
| if (logger.isDebugEnabled()) { | ||
| logger.debug("Rolling back transaction because of listener exception thrown: " + ex); | ||
|
|
@@ -339,8 +337,7 @@ protected boolean doReceiveAndExecute(Object invoker, @Nullable Session session, | |
| } | ||
| try { | ||
| handleListenerException(ex); | ||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please revert this new line change. |
||
| catch (Throwable throwable) { | ||
| } catch (Throwable throwable) { | ||
| observation.error(throwable); | ||
| throw throwable; | ||
| } | ||
|
|
@@ -349,13 +346,11 @@ protected boolean doReceiveAndExecute(Object invoker, @Nullable Session session, | |
| if (ex instanceof JMSException jmsException) { | ||
| throw jmsException; | ||
| } | ||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please revert this new line change. |
||
| finally { | ||
| } finally { | ||
| if (exposeResource) { | ||
| TransactionSynchronizationManager.unbindResource(obtainConnectionFactory()); | ||
| } | ||
| observation.stop(); | ||
| scope.close(); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not a neutral change. The scope is now closed before the observation is stopped and the transaction finished. This will change the behavior and will have unintended consequences. |
||
| } | ||
| // Indicate that a message has been received. | ||
| return true; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -152,22 +152,16 @@ protected void writeContent(Resource resource, HttpOutputMessage outputMessage) | |
| // We cannot use try-with-resources here for the InputStream, since we have | ||
| // custom handling of the close() method in a finally-block. | ||
| try { | ||
| InputStream in = resource.getInputStream(); | ||
| try { | ||
| OutputStream out = outputMessage.getBody(); | ||
| in.transferTo(out); | ||
| out.flush(); | ||
| } | ||
| catch (NullPointerException ex) { | ||
| // ignore, see SPR-13620 | ||
| } | ||
| finally { | ||
| try (InputStream in = resource.getInputStream()) { | ||
| try { | ||
| in.close(); | ||
| } | ||
| catch (Throwable ex) { | ||
| // ignore, see SPR-12999 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please revert. This will catch all Throwable thrown by the code block, not just when the resource is closed. |
||
| OutputStream out = outputMessage.getBody(); | ||
| in.transferTo(out); | ||
| out.flush(); | ||
| } catch (NullPointerException ex) { | ||
| // ignore, see SPR-13620 | ||
| } | ||
| } catch (Throwable ex) { | ||
| // ignore, see SPR-12999 | ||
| } | ||
| } | ||
| catch (FileNotFoundException ex) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -173,19 +173,11 @@ protected void writeResourceRegion(ResourceRegion region, HttpOutputMessage outp | |
| responseHeaders.add("Content-Range", "bytes " + start + '-' + end + '/' + resourceLength); | ||
| responseHeaders.setContentLength(rangeLength); | ||
|
|
||
| InputStream in = region.getResource().getInputStream(); | ||
| // We cannot use try-with-resources here for the InputStream, since we have | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is literally called out in this comment...
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes need to stay open i know this behavior from maven. should give more detail then as ignored exc for good reason. |
||
| // custom handling of the close() method in a finally-block. | ||
| try { | ||
| try (InputStream in = region.getResource().getInputStream()) { | ||
| StreamUtils.copyRange(in, outputMessage.getBody(), start, end); | ||
| } | ||
| finally { | ||
| try { | ||
| in.close(); | ||
| } | ||
| catch (IOException ex) { | ||
| // ignore | ||
| } | ||
| } catch (IOException ignored) { | ||
| } | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't change files in the cglib package, this part is manually shaded into our repository and we shouldn't change the source if we want to easily keep up with the original.