commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Damjan Jovanovic <damjan....@gmail.com>
Subject Re: [imaging] Closing stream
Date Wed, 30 Oct 2013 18:51:50 GMT
For the sake of similarity of 1.0 with 0.x versions, I've stayed with Java
1.5 and in commit 1537238 I've gone with the set flag on every successful
return path + closeQuietly() idea.

It was a mission and broke things (DeflaterOutputStream apparently doesn't
honor flush()). I am moving Imaging to Java 7 as soon as 1.0 is released.

Damjan


On Sun, Oct 27, 2013 at 8:32 PM, Gary Gregory <garydgregory@gmail.com>wrote:

> On Sun, Oct 27, 2013 at 1:51 AM, Damjan Jovanovic <damjan.jov@gmail.com
> >wrote:
>
> >  an
> >
> > On Fri, Oct 25, 2013 at 5:24 PM, Jörg Schaible <joerg.schaible@gmx.de>
> > wrote:
> > > Hi Damjan,
> > >
> > > Damjan Jovanovic wrote:
> > >
> > >> On Fri, Oct 25, 2013 at 12:36 PM, Jörg Schaible
> > >> <Joerg.Schaible@scalaris.com> wrote:
> > >>> Hi Damjan,
> > >>>
> > >>> Damjan Jovanovic wrote:
> > >>>
> > >>> [snip]
> > >>>
> > >>> Thanks for explanation.
> > >>>
> > >>>> We would be able to adapt that for Java < 1.7 by swallowing
the
> close
> > >>>> exception instead of calling addSuppressed() on the primary
> exception,
> > >>>> but the show stopper is catching and rethrowing the primary
> exception
> > >>>> (Throwable), which javac < 1.7 refuses to compile because it
doesn't
> > >>>> do "Rethrowing Exceptions with More Inclusive Type Checking"
> > >>>> (
> >
> http://docs.oracle.com/javase/7/docs/technotes/guides/language/catch-multiple.html
> > ).
> > >>>>
> > >>>> But this would work and always sets succeeded correctly without
> > >>>> catch/re-throw:
> > >>>>
> > >>>> final InputStream is = factoryMethodThatCanThrow();
> > >>>> boolean succeeded = false;
> > >>>> try {
> > >>>>     try {
> > >>>>         is.methodThatCanThrow();
> > >>>>     } finally {
> > >>>>     }
> > >>>>     succeeded = true;
> > >>>> } finally {
> > >>>>     closeSafely(!succeeded, is);
> > >>>> }
> > >>>
> > >>> I guess the nested try was unintentionally ;-)
> > >>>
> > >>> Cheers,
> > >>> Jörg
> > >>
> > >> Well that actually won't work, because the "succeeded = true;" will be
> > >> skipped if there is a "return;" in the inner try.
> > >
> > > Well, but this has to be done in our code, so we can either change it
> or
> > set
> > > "succedded = true" before the return as well.
> > >
> > > To mimic Java 7, we could also implement:
> > >
> > > ============= %< ===============
> > >  Throwable t = null;
> > >  try {
> > >  } (catch IOException e) { t = e; throw e; }
> > >  // ... another line for each checked exception
> > >  } (catch RuntimeException e) { t = e; throw e; }
> > >  } (catch Error e) { t = e; throw e; }
> > >  } finally {
> > >    closeSafely(t != null, is);
> > >  }
> > > ============= %< ===============
> > >
> > > but as commented, we have to add a catch for every checked exception,
> > > anotehr one for RuntimeException and one for Error. The approach with
> the
> > > succeeded flag seems easier ...
> > >
> > >> Other than a custom Java compiler, I guess there's no clean way of
> > >> doing this in Java < 1.7. There's really only option 2 - with being
> > >> careful to always set succeeded correctly on all paths out of the try
> > >> block. Almost like releasing memory in C.
> > >
> > > Yep.
> > >
> > > Cheers,
> > > Jörg
> > >
> >
> > One thing that amuses me to no end, is that while it's at least a
> > solved problem in Java 7, exceptions thrown from C#'s Dispose() method
> > in a using() block always swallow the original exception, just like an
> > uncaught close() exception in Java's finally :)
> > (
> >
> http://blogs.infosupport.com/the-c-using-statement-and-suppressed-exceptions-compared-to-java-7-try-with/
> > ).
> >
> > Anyway I now think the way forward is Java 7. Java 5 was EOL since 3
> > November 2009, and Java 6 was EOL since February 2013 (with a last
> > update on 6 March 2013). There are ways of getting Java 7 features
> > like try-with-resources on Android
> > (https://github.com/yareally/Java7-on-Android) and besides Imaging's
> > use of java.awt.* packages is a bigger barrier there. Applications and
> > JVMs will eventually support Java 7 anyway, and even if they don't, a
> > special compiler could produce binaries for earlier versions of Java.
> >
> > Can I just go change the POM to Java 7 or do we need a vote?
> >
>
> I do not think you need a vote. Since the 0.x version is used in the wild,
> you might want to poll the user base as to their JRE requirements.
>
> For me, moving to Java 6 is fine. Java 7 is a little more adventurous, a
> good thing in general since no [commons] component has made Java 7 a
> minimum yet. Might as well test the waters.
>
> Gary
>
>
>
> >
> > Damjan
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > For additional commands, e-mail: dev-help@commons.apache.org
> >
> >
>
>
> --
> E-Mail: garydgregory@gmail.com | ggregory@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
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message