[jboss-jira] [JBoss JIRA] Updated: (JBAS-2910) minor try-finally changes to make sure that some streams and JDBC objects are always closed

minor try-finally changes to make sure that some streams and JDBC objects are always closed ------------------------------------------------------------------------------------------- Key: JBAS-2910 URL: http://jira.jboss.com/jira/browse/JBAS-2910 Project: JBoss Application Server Issue Type: Patch Security Level: Public(Everyone can see) Components: Web (Tomcat) service, CMP service Reporter: Westley Weimer Priority: Minor Fix For: JBossAS-4.2.0.GA Original Estimate: 20 minutes Remaining Estimate: 20 minutes Attached are minor candidate patches for jboss (against a CVS checkout of the sources on 2006 03 08) that ensure that resources are closed properly in some unlikely exceptional cases. These changse should not affect jboss functionality at all aside from ensuring that resources are closed in a few more cases. This is hardly the most exciting sort of bugfix in the world, but jboss does seem to be concerned with leaks and being ironclad in these situations. The patches all have the general flavor of changing "open; work; close" to "try { open; work; } finally { close; }". It should take a knowledgeable dev at most a few minutes to eyeball them and accept or reject them. I'm not sure how to attach the patch file, so I'm copying it here. My apologies if this is the wrong place for this sort of thing. --- jboss-20060308-orig/src/main/org/jboss/ejb/plugins/CMPInMemoryPersistenceManager.java 2006-03-08 16:29:45.917109700 -0500 +++ jboss-20060308/src/main/org/jboss/ejb/plugins/CMPInMemoryPersistenceManager.java 2006-03-08 16:34:15.141058700 -0500 @@ -355,11 +355,12 @@ */ public void loadEntity (EntityEnterpriseContext ctx) { + java.io.ObjectInputStream in = null; try { // Read fields - java.io.ObjectInputStream in = new CMPObjectInputStream + in = new CMPObjectInputStream (new java.io.ByteArrayInputStream ((byte[])this.beans.get (ctx.getId ()))); Object obj = ctx.getInstance (); @@ -370,12 +371,15 @@ f[i].set (obj, in.readObject ()); } - in.close (); } catch (Exception e) { throw new EJBException ("Load failed", e); } + finally + { + if (in != null) { try { in.close(); } catch (Exception e) { } } + } } /** --- jboss-20060308-orig/src/main/org/jboss/invocation/http/interfaces/Util.java 2006-03-08 16:29:47.899706700 -0500 +++ jboss-20060308/src/main/org/jboss/invocation/http/interfaces/Util.java 2006-03-08 17:32:40.259780600 -0500 @@ -158,7 +158,9 @@ conn.setRequestProperty("ContentType", REQUEST_CONTENT_TYPE); conn.setRequestMethod("POST"); OutputStream os = conn.getOutputStream(); + ObjectInputStream ois = null; ObjectOutputStream oos = new ObjectOutputStream(os); + try { try { oos.writeObject(mi); @@ -173,12 +175,14 @@ // Get the response MarshalledValue object InputStream is = conn.getInputStream(); - ObjectInputStream ois = new ObjectInputStream(is); + ois = new ObjectInputStream(is); MarshalledValue mv = (MarshalledValue) ois.readObject(); // A hack for jsse connection pooling (see patch ). ois.read(); - ois.close(); - oos.close(); + } finally { + if (ois != null) { try { ois.close(); } catch (Exception e) { } } + if (oos != null) { try { oos.close(); } catch (Exception e) { } } + } // If the encoded value is an exception throw it Object value = mv.get(); --- jboss-20060308-orig/src/main/org/jboss/invocation/MarshalledValue.java 2006-03-08 16:29:48.008983700 -0500 +++ jboss-20060308/src/main/org/jboss/invocation/MarshalledValue.java 2006-03-08 17:30:01.198298600 -0500 @@ -89,9 +89,12 @@ ByteArrayInputStream bais = new ByteArrayInputStream(serializedForm); MarshalledValueInputStream mvis = new MarshalledValueInputStream(bais); + try { Object retValue = mvis.readObject(); - mvis.close(); return retValue; + } finally { + mvis.close(); + } } public byte[] toByteArray() --- jboss-20060308-orig/src/main/org/jboss/invocation/pooled/interfaces/PooledMarshalledValue.java 2006-03-08 16:29:48.055816700 -0500 +++ jboss-20060308/src/main/org/jboss/invocation/pooled/interfaces/PooledMarshalledValue.java 2006-03-08 17:34:20.946787700 -0500 @@ -90,9 +90,12 @@ ByteArrayInputStream bais = new ByteArrayInputStream(serializedForm); ObjectInputStream mvis = new OptimizedObjectInputStream(bais); + try { Object retValue = mvis.readObject(); - mvis.close(); return retValue; + } finally { + try { mvis.close(); } catch (Exception e) { } + } } public byte[] toByteArray() --- jboss-20060308-orig/src/main/org/jboss/naming/HttpNamingContextFactory.java 2006-03-08 16:29:48.914421700 -0500 +++ jboss-20060308/src/main/org/jboss/naming/HttpNamingContextFactory.java 2006-03-08 17:35:39.213414500 -0500 @@ -132,8 +132,12 @@ InputStream is = conn.getInputStream(); ObjectInputStream ois = new ObjectInputStream(is); - MarshalledValue mv = (MarshalledValue) ois.readObject(); - ois.close(); + MarshalledValue mv = null; + try { + mv = (MarshalledValue) ois.readObject(); + } finally { + try { ois.close(); } catch (Exception e) { } + } Object obj = mv.get(); if( (obj instanceof Naming) == false ) --- jboss-20060308-orig/src/main/org/jboss/web/AbstractWebContainer.java 2006-03-08 16:29:49.413973700 -0500 +++ jboss-20060308/src/main/org/jboss/web/AbstractWebContainer.java 2006-03-08 17:37:40.449341300 -0500 @@ -322,8 +322,11 @@ throw new DeploymentException("Was unable to mkdir: "+expWarFile); log.debug("Unpacking war to: "+expWarFile); FileInputStream fis = new FileInputStream(warFile); + try { JarUtils.unjar(fis, expWarFile); - fis.close(); + } finally { + try { fis.close(); } catch (Exception e) { } + } log.debug("Replaced war with unpacked contents"); if (warFile.delete() == false) log.debug("Was unable to delete war file"); @@ -416,6 +419,7 @@ FileInputStream fis = new FileInputStream(warFile); JarInputStream jin = new JarInputStream(fis); + try { ZipEntry entry = jin.getNextEntry(); while (entry != null) { @@ -426,8 +430,9 @@ } entry = jin.getNextEntry(); } - jin.close(); - + } finally { + try { jin.close(); } catch (Exception e) { } + } URL[] urlArr = new URL[urlList.size()]; urlList.toArray(urlArr); di.annotationsCl = new URLClassLoader(urlArr, di.ucl); @@ -706,6 +711,7 @@ // First check for a WEB-INF/web.xml and a WEB-INF/jboss-web.xml InputStream warIS = warURL.openStream(); java.util.zip.ZipInputStream zipIS = new java.util.zip.ZipInputStream(warIS); + try { java.util.zip.ZipEntry entry; byte[] buffer = new byte[512]; int bytes; @@ -730,7 +736,9 @@ jbossWebIS = new ByteArrayInputStream(baos.toByteArray()); } } - zipIS.close(); + } finally { + try { zipIS.close(); } catch (Exception e) { } + } } XmlFileLoader xmlLoader = new XmlFileLoader(); --- jboss-20060308-orig/src/main/org/jboss/web/WebServer.java 2006-03-08 16:29:49.445195700 -0500 +++ jboss-20060308/src/main/org/jboss/web/WebServer.java 2006-03-08 17:38:28.403080100 -0500 @@ -1,4 +1,4 @@ -/* +/* } catch (Exception e) { } * JBoss, Home of Professional Open Source * Copyright 2005, JBoss Inc., and individual contributors as indicated * by the @authors tag. See the copyright.txt in the distribution for a @@ -521,6 +521,7 @@ protected byte[] getBytes(URL url) throws IOException { InputStream in = new BufferedInputStream(url.openStream()); + try { log.debug("Retrieving " + url); ByteArrayOutputStream out = new ByteArrayOutputStream(); byte[] tmp = new byte[1024]; @@ -529,8 +530,10 @@ { out.write(tmp, 0, bytes); } - in.close(); return out.toByteArray(); + } finally { + try { in.close(); } catch (Exception e) { } + } } /** --- jboss-20060308-orig/src/main/org/jboss/ejb/plugins/cmp/jdbc/keygen/JDBCDB2IdentityValLocalCreateCommand.java 2006-03-08 16:29:45.323891700 -0500 +++ jboss-20060308/src/main/org/jboss/ejb/plugins/cmp/jdbc/keygen/JDBCDB2IdentityValLocalCreateCommand.java 2006-03-08 17:19:47.443491800 -0500 @@ -44,8 +44,9 @@ protected int executeInsert(int paramIndex, PreparedStatement ps, EntityEnterpriseContext ctx ) throws SQLException { int rows = ps.executeUpdate(); ResultSet results = null; + Connection conn = null; try { - Connection conn = ps.getConnection(); + conn = ps.getConnection(); results = conn.prepareStatement( SQL ).executeQuery(); if( !results.next() ) { throw new EJBException( "identity_val_local() returned an empty ResultSet" ); @@ -58,6 +59,7 @@ throw new EJBException( "Error extracting identity_val_local()", e ); } finally { JDBCUtil.safeClose( results ); + JDBCUtil.safeClose(con); } return rows; } --- jboss-20060308-orig/src/main/org/jboss/ejb/plugins/cmp/jdbc/keygen/JDBCPostgreSQLCreateCommand.java 2006-03-08 16:29:45.355113700 -0500 +++ jboss-20060308/src/main/org/jboss/ejb/plugins/cmp/jdbc/keygen/JDBCPostgreSQLCreateCommand.java 2006-03-08 17:22:01.854042800 -0500 @@ -74,11 +74,12 @@ Statement s = null; ResultSet rs = null; + Connection c = null; try { if (trace) { log.trace("Executing SQL :"+sequenceSQL); } - Connection c = ps.getConnection(); + c = ps.getConnection(); s = c.createStatement(); rs = s.executeQuery(sequenceSQL); if (!rs.next()) { @@ -93,6 +94,7 @@ } finally { JDBCUtil.safeClose(rs); JDBCUtil.safeClose(s); + JDBCUtil.safeClose(c); } return rows; --- jboss-20060308-orig/src/main/org/jboss/ejb/plugins/cmp/jdbc/keygen/JDBCSQLServerCreateCommand.java 2006-03-08 16:29:45.355113700 -0500 +++ jboss-20060308/src/main/org/jboss/ejb/plugins/cmp/jdbc/keygen/JDBCSQLServerCreateCommand.java 2006-03-08 17:23:29.513540100 -0500 @@ -61,10 +61,10 @@ protected int executeInsert(int index, PreparedStatement ps, EntityEnterpriseContext ctx) throws SQLException { - ps.execute(); ResultSet rs = null; try { + ps.execute(); int rows = ps.getUpdateCount(); if(rows != 1) { @@ -95,6 +95,7 @@ finally { JDBCUtil.safeClose(rs); + JDBCUtil.safeClose(ps); } } }