You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2019/04/17 16:55:31 UTC

[tomcat] branch master updated: Fix resource leak on exception path

This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/master by this push:
     new d0eed1e  Fix resource leak on exception path
d0eed1e is described below

commit d0eed1e1e1f2cdd050426aa176a2ba1efb86735f
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Apr 17 17:55:16 2019 +0100

    Fix resource leak on exception path
    
    Identified by Coverity scan
---
 java/org/apache/naming/factory/LookupFactory.java | 8 ++++++++
 webapps/docs/changelog.xml                        | 4 ++++
 2 files changed, 12 insertions(+)

diff --git a/java/org/apache/naming/factory/LookupFactory.java b/java/org/apache/naming/factory/LookupFactory.java
index ee239ad..6e7455c 100644
--- a/java/org/apache/naming/factory/LookupFactory.java
+++ b/java/org/apache/naming/factory/LookupFactory.java
@@ -134,6 +134,14 @@ public class LookupFactory implements ObjectFactory {
                             name, ref.getClassName(), lookupName, result.getClass().getName());
                     NamingException ne = new NamingException(msg);
                     log.warn(msg, ne);
+                    // Close the resource we no longer need if we know how to do so
+                    if (result instanceof AutoCloseable) {
+                        try {
+                            ((AutoCloseable) result).close();
+                        } catch (Exception e) {
+                            // Ignore
+                        }
+                    }
                     throw ne;
                 }
             } finally {
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 8de445d..1a9f224 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -76,6 +76,10 @@
         Fix a potential resource leak on an exception path when parsing JSP
         files. Identified by Coverity scan. (markt)
       </fix>
+      <fix>
+        Fix a potential resource leak when a JNDI lookup returns an object of an
+        in compatible class. Identified by Coverity scan. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: [tomcat] branch master updated: Fix resource leak on exception path

Posted by Mark Thomas <ma...@apache.org>.
On 17/04/2019 19:22, Christopher Schultz wrote:
> On 4/17/19 12:55, markt@apache.org wrote:

<snip/>

>> Close the resource we no longer need if we know how to do so +
>> if (result instanceof AutoCloseable) { +                        try
>> { +                            ((AutoCloseable) result).close(); +
>> } catch (Exception e) { +                            // Ignore +
>> } +                    }
> 
> Any concrete objections to logging these exceptions?

No.

> Swallowing
> exceptions is almost never a good practice, and if they are happening
> it's probably worth knowing something is amiss.

Yeah. I know. I was short on time (still am) and calling close and not
logging the exception was better than what was there before. Going back
over these and adding some log message is on my TODO list. I'm not going
to object if someone beats me to it ;)

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: [tomcat] branch master updated: Fix resource leak on exception path

Posted by Christopher Schultz <ch...@christopherschultz.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Mark,

On 4/17/19 12:55, markt@apache.org wrote:
> This is an automated email from the ASF dual-hosted git
> repository.
> 
> markt pushed a commit to branch master in repository
> https://gitbox.apache.org/repos/asf/tomcat.git
> 
> 
> The following commit(s) were added to refs/heads/master by this
> push: new d0eed1e  Fix resource leak on exception path d0eed1e is
> described below
> 
> commit d0eed1e1e1f2cdd050426aa176a2ba1efb86735f Author: Mark Thomas
> <ma...@apache.org> AuthorDate: Wed Apr 17 17:55:16 2019 +0100
> 
> Fix resource leak on exception path
> 
> Identified by Coverity scan --- 
> java/org/apache/naming/factory/LookupFactory.java | 8 ++++++++ 
> webapps/docs/changelog.xml                        | 4 ++++ 2 files
> changed, 12 insertions(+)
> 
> diff --git a/java/org/apache/naming/factory/LookupFactory.java
> b/java/org/apache/naming/factory/LookupFactory.java index
> ee239ad..6e7455c 100644 ---
> a/java/org/apache/naming/factory/LookupFactory.java +++
> b/java/org/apache/naming/factory/LookupFactory.java @@ -134,6
> +134,14 @@ public class LookupFactory implements ObjectFactory { 
> name, ref.getClassName(), lookupName,
> result.getClass().getName()); NamingException ne = new
> NamingException(msg); log.warn(msg, ne); +                    //
> Close the resource we no longer need if we know how to do so +
> if (result instanceof AutoCloseable) { +                        try
> { +                            ((AutoCloseable) result).close(); +
> } catch (Exception e) { +                            // Ignore +
> } +                    }

Any concrete objections to logging these exceptions? Swallowing
exceptions is almost never a good practice, and if they are happening
it's probably worth knowing something is amiss.

- -chris
-----BEGIN PGP SIGNATURE-----
Comment: Using GnuPG with Thunderbird - https://www.enigmail.net/

iQIzBAEBCAAdFiEEMmKgYcQvxMe7tcJcHPApP6U8pFgFAly3bvkACgkQHPApP6U8
pFit1A/+J0ST0SvBn4+2qqX4/Vs0kHPN4z8Uqhn2VVW6Byv1zSSZKmFB/x2/ucRa
mUEMUo9XPJ2GaXP2onfMCE5iPcObF+vHYNAvsc1Y3PNghRpGtxgxC2e0hpTIKeKS
YOXULHjN/Y5Tr0zSpDcnvqMUqehIaLpJv2EZkHmIMa1Z/nDHenT63nS0t5DlEoSf
7IoLhAuDUJt2auUJF3U4olN0ouplMMwdSxDq0KjLGNJWDNgiZnbb7XMGKAPjnzUb
Cm3aOxTrAlOvk0dmy++0Nsbr05WJ+pSo2l9m2C++AYR/bTTalmmV4zgaZmcV5zpz
9EMHG11QszKfXs1ojLayp+cEKqKdHwz+Hp/CdjuUKuyucxmgVz5EmrosJrNiAlhY
oFFTpaftoSShrz6Tgx3Y3TLptx75MH4sRhtnuSJJjytB72HM+rMV4agEqX8jbZcX
vWenFtJCpmrUI0nHM7xaTvA0vWg9Z6Xt3yv11C44pV3z7DvS6fldmW2AFQVJe3Ep
H300FMoHI5Y1RpsAoXwKT0ptWfmc9vsf0JgoI2oVGwQYm7mYkdYed5I97eJXnqgk
SOn9J/alFfC1inRZItXfJUDJvh4hUJZEHv9OcQ27W0+aOfRLyEdHmpn5S4EXIOwm
/hU6vUzmogEpMC5vozTlJBV1ltGax3JOgGFdWi1ovvJG560Ccv8=
=saWD
-----END PGP SIGNATURE-----

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org