use `try-with-resources` statement #35046

Pankraz76 · 2025-06-13T09:36:14Z

Java 7 introduced the try-with-resources statement. This statement ensures that each resource is closed at the end of the statement. It avoids the need of explicitly closing the resources in a finally block. Additionally exceptions are better handled: If an exception occurred both in the try block and finally block, then the exception from the try block was suppressed. With the try-with-resources statement, the exception thrown from the try-block is preserved.

.../src/main/java/org/springframework/jms/listener/AbstractPollingMessageListenerContainer.java

Signed-off-by: Vincent Potucek <vpotucek@me.com>

bclozel

Thanks for raising this, but as this review shows, these stylistic changes are mostly breaking the behavior or adding little value.

bclozel · 2025-06-13T09:49:39Z

spring-core/src/main/java/org/springframework/cglib/core/DuplicatesPredicate.java

            continue;
          }
          InputStream is = cl.getResourceAsStream(c.getName().replace('.', '/') + ".class");
-          if (is == null) {


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.

bclozel · 2025-06-13T09:49:48Z

spring-core/src/main/java/org/springframework/cglib/proxy/BridgeMethodResolver.java

            Set bridges = (Set) entry.getValue();
            try {
                InputStream is = classLoader.getResourceAsStream(owner.getName().replace('.', '/') + ".class");
-                if (is == null) {


bclozel · 2025-06-13T09:49:55Z

spring-core/src/main/java/org/springframework/cglib/transform/AbstractClassLoader.java

                       name.replace('.','/') + ".class"
                  );

-           if (is == null) {


bclozel · 2025-06-13T09:51:06Z

spring-core/src/main/java/org/springframework/core/io/AbstractResource.java

 			}
 			return size;
 		}
-		finally {


Please revert. The exception thrown while closing the stream is not caught/logged anymore.
See this message for a discussion on possible workarounds. I think we should keep the current code.

bclozel · 2025-06-13T09:51:20Z

...in/java/org/springframework/jdbc/datasource/embedded/AbstractEmbeddedDatabaseConfigurer.java

-				try {
-					con.close();
-				}
-				catch (SQLException ex) {


Same. Please revert.

bclozel · 2025-06-13T09:55:09Z

.../src/main/java/org/springframework/jms/listener/AbstractPollingMessageListenerContainer.java

 					}
 					try {
 						handleListenerException(ex);
-					}


please revert this new line change.

bclozel · 2025-06-13T09:55:22Z

.../src/main/java/org/springframework/jms/listener/AbstractPollingMessageListenerContainer.java

 					if (ex instanceof JMSException jmsException) {
 						throw jmsException;
 					}
-				}


please revert this new line change.

bclozel · 2025-06-13T09:56:30Z

.../src/main/java/org/springframework/jms/listener/AbstractPollingMessageListenerContainer.java

 						TransactionSynchronizationManager.unbindResource(obtainConnectionFactory());
 					}
 					observation.stop();
-					scope.close();


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.
Please revert.

bclozel · 2025-06-13T09:57:33Z

spring-web/src/main/java/org/springframework/http/converter/ResourceHttpMessageConverter.java

-					in.close();
-				}
-				catch (Throwable ex) {
-					// ignore, see SPR-12999


Please revert. This will catch all Throwable thrown by the code block, not just when the resource is closed.

bclozel · 2025-06-13T09:58:13Z

...web/src/main/java/org/springframework/http/converter/ResourceRegionHttpMessageConverter.java

 		responseHeaders.setContentLength(rangeLength);

-		InputStream in = region.getResource().getInputStream();
 		// We cannot use try-with-resources here for the InputStream, since we have


This is literally called out in this comment...

yes need to stay open i know this behavior from maven. should give more detail then as ignored exc for good reason.

fix ignored exceptions #35047

bclozel · 2025-06-13T09:59:47Z

Given the state of this PR, I don't think it's worth spending time on this - we're more likely to break behavior than anything.

Pankraz76 · 2025-06-13T10:03:47Z

yes of course will SOC. if this changes anything its a interesting to invest, as its a valid feature used all around already in this code base:

spring-framework/spring-context/src/main/java/org/springframework/context/support/ResourceBundleMessageSource.java

Line 414 in d8ac3ff

try (InputStream bundleStream = inputStream) {

Pankraz76 · 2025-06-13T10:04:54Z

if can not use java like supposed to, it seems smell.

Pankraz76

thx. for detailed review. yes this need special attention.

Pankraz76 · 2025-06-13T10:09:16Z

...web/src/main/java/org/springframework/http/converter/ResourceRegionHttpMessageConverter.java

 		responseHeaders.setContentLength(rangeLength);

-		InputStream in = region.getResource().getInputStream();
 		// We cannot use try-with-resources here for the InputStream, since we have


yes need to stay open i know this behavior from maven. should give more detail then as ignored exc for good reason.

fix ignored exceptions #35047

Pankraz76 commented Jun 13, 2025

View reviewed changes

.../src/main/java/org/springframework/jms/listener/AbstractPollingMessageListenerContainer.java Show resolved Hide resolved

Pankraz76 mentioned this pull request Jun 13, 2025

use try-with-resources statement openrewrite/rewrite-static-analysis#591

Open

spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jun 13, 2025

Pankraz76 force-pushed the use-try-with-resources-statement- branch from 4e2e1be to 9aa6812 Compare June 13, 2025 09:52

use try-with-resources statement

d8d4b6e

Signed-off-by: Vincent Potucek <vpotucek@me.com>

Pankraz76 force-pushed the use-try-with-resources-statement- branch from 9aa6812 to d8d4b6e Compare June 13, 2025 09:54

bclozel requested changes Jun 13, 2025

View reviewed changes

bclozel closed this Jun 13, 2025

bclozel added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jun 13, 2025

Pankraz76 commented Jun 13, 2025

View reviewed changes

This was referenced Jun 13, 2025

fix ignored exceptions #35047

Closed

use try-with-resources statement quarkusio/quarkus#48364

Closed

TryWithResources recipe openrewrite/rewrite-static-analysis#593

Closed

use try-with-resources statement #35046

use try-with-resources statement #35046

Uh oh!

Conversation

Pankraz76 commented Jun 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

bclozel left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bclozel commented Jun 13, 2025

Uh oh!

Pankraz76 commented Jun 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Pankraz76 commented Jun 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Pankraz76 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

use `try-with-resources` statement #35046

use `try-with-resources` statement #35046

Pankraz76 commented Jun 13, 2025 •

edited

Loading

Pankraz76 commented Jun 13, 2025 •

edited

Loading

Pankraz76 commented Jun 13, 2025 •

edited

Loading