On Thu, Aug 22, 2013 at 8:31 PM, Remko Popma <remko.po...@gmail.com> wrote:
> Sorry if I was unclear. What if creating the Reader fails after the > InputStream was created? Then there is no reader object to call close on... > In not sure that protecting against a NPE is enough, so I want to close the > InputStream in the finally clause. Closing the Reader as well is fine but > optional IMO. > OK, I can see that. Since each JRE can implement the reader differently, is should also be closed. This would be easier in Java 7 with a try-with-resources, so now I have: > On Friday, August 23, 2013, Gary Gregory wrote: > >> Hi Remko, >> >> I tried it both ways and it just looks simpler to read this way. The >> reader passes the close to its wrapped stream. In both cases, you need to >> close the reader (or the stream) and protect that close with a null guard. >> >> Gary >> >> >> On Thu, Aug 22, 2013 at 7:10 PM, Remko Popma <remko.po...@gmail.com>wrote: >> >> Gregory, >> I'm not sure I agree with this fix. >> NPE is not the only reason why creating the reader could fail. >> The input stream is the object associated with the native IO resource >> (the file handle). As such, it is safer to guarantee that the input stream >> is always closed no matter what happens. The Reader is just a java object >> with some char buffer space, and these resources will be GC'ed when it goes >> out of scope. >> >> Remko >> >> PS >> >> Check out Assert#isNotNull in the helpers package. I like using this >> instead of an if statement: short, clearly shows intent, and can be used in >> assignment statements (often convenient in constructors). >> >> On Thursday, August 22, 2013, wrote: >> >> Author: ggregory >> Date: Thu Aug 22 14:58:44 2013 >> New Revision: 1516477 >> >> URL: http://svn.apache.org/r1516477 >> Log: >> Make sure no resources are leaked managing streams and readers. >> >> Modified: >> >> logging/log4j/log4j2/trunk/core/src/main/java/org/apache/logging/log4j/core/jmx/LoggerContextAdmin.java >> >> Modified: >> logging/log4j/log4j2/trunk/core/src/main/java/org/apache/logging/log4j/core/jmx/LoggerContextAdmin.java >> URL: >> http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/core/src/main/java/org/apache/logging/log4j/core/jmx/LoggerContextAdmin.java?rev=1516477&r1=1516476&r2=1516477&view=diff >> >> ============================================================================== >> --- >> logging/log4j/log4j2/trunk/core/src/main/java/org/apache/logging/log4j/core/jmx/LoggerContextAdmin.java >> (original) >> +++ >> logging/log4j/log4j2/trunk/core/src/main/java/org/apache/logging/log4j/core/jmx/LoggerContextAdmin.java >> Thu Aug 22 14:58:44 2013 >> @@ -32,6 +32,7 @@ import java.nio.charset.Charset; >> import java.util.Map; >> import java.util.concurrent.Executor; >> import java.util.concurrent.atomic.AtomicLong; >> + >> import javax.management.MBeanNotificationInfo; >> import javax.management.Notification; >> import javax.management.NotificationBroadcasterSupport; >> @@ -186,11 +187,22 @@ public class LoggerContextAdmin extends >> } >> } >> >> + /** >> + * >> + * @param uri >> + * @param charset MUST not be null >> + * @return >> + * @throws IOException >> + */ >> private String readContents(final URI uri, final Charset charset) >> throws IOException { >> - InputStream in = null; >> + if (charset == null) { >> + throw new IllegalArgumentException("charset must not be >> null"); >> + } >> + Reader reader = null; >> try { >> - in = uri.toURL().openStream(); >> - final Reader reader = new InputStreamReader(in, charset); >> + InputStream in = uri.toURL().openStream(); >> + // The stream is open and charset is not null, we can create >> the reade >> >> -- >> E-Mail: garydgreg...@gmail.com | ggreg...@apache.org >> Java Persistence with Hibernate, Second >> Edition<http://www.manning.com/bauer3/> >> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/> >> Spring Batch in Action <http://www.manning.com/templier/> >> Blog: http://garygregory.wordpress.com >> Home: http://garygregory.com/ >> Tweet! http://twitter.com/GaryGregory >> > -- E-Mail: garydgreg...@gmail.com | ggreg...@apache.org Java Persistence with Hibernate, Second Edition<http://www.manning.com/bauer3/> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/> Spring Batch in Action <http://www.manning.com/templier/> Blog: http://garygregory.wordpress.com Home: http://garygregory.com/ Tweet! http://twitter.com/GaryGregory
