You are viewing a plain text version of this content. The canonical link for it is here.
Posted to user@tika.apache.org by Michael McCandless <lu...@mikemccandless.com> on 2011/09/01 11:36:38 UTC
Re: Tika leaves files open
Actually Tika already has the method to do this: IOUtils.closeQuietly.
I think we should use it here?
While rare for IS.close() to throw an exception, if it does, it's
quite awful because it masks the original exception. It seems best to
be defensive?
Mike McCandless
http://blog.mikemccandless.com
On Wed, Aug 31, 2011 at 1:25 PM, Jukka Zitting <ju...@gmail.com> wrote:
> Hi,
>
> On Wed, Aug 31, 2011 at 6:53 PM, Uwe Schindler <uw...@thetaphi.de> wrote:
>> For this typical try...finally code I suggest to use a pre-Java-7.0
>> workaround to make this behave more correct: If you do try...finally and
>> another Exception occurs on close() in the finally block, you lose the first
>> Exception.
>
> Such a case is certainly possible scenario, but in my experience it
> practically never occurs. I've used the try { ... } finally {
> stream.close(); } pattern extensively for years, and I've never seen a
> case where information was lost because of this.
>
> So personally I consider the problem rather theoretical and would
> rather opt for cleaner code that avoids the extra constructs.
>
> BR,
>
> Jukka Zitting
>
RE: Tika leaves files open
Posted by Uwe Schindler <uw...@thetaphi.de>.
This is why we use the "modernized" Lucene IOUtils.closeSafely... (and
because Lucene has no external references).
-----
Uwe Schindler
H.-H.-Meier-Allee 63, D-28213 Bremen
http://www.thetaphi.de
eMail: uwe@thetaphi.de
> -----Original Message-----
> From: Jukka Zitting [mailto:jukka.zitting@gmail.com]
> Sent: Thursday, September 01, 2011 11:56 AM
> To: user@tika.apache.org
> Subject: Re: Tika leaves files open
>
> Hi,
>
> On Thu, Sep 1, 2011 at 11:36 AM, Michael McCandless
> <lu...@mikemccandless.com> wrote:
> > While rare for IS.close() to throw an exception, if it does, it's
> > quite awful because it masks the original exception. It seems best to
> > be defensive?
>
> With closeQuietly() you'd really be throwing information out in case where
> close() fails when no other exception has been thrown. Instead of one
exception
> masking another, you'd have no exceptions masking one!
>
> BR,
>
> Jukka Zitting
RE: Tika leaves files open
Posted by Uwe Schindler <uw...@thetaphi.de>.
With our internal Lucene IOUtils it's even simplier, see javadocs :-) This
is why I proposed to use it also for TIKA:
Closeable resource1 = null, resource2 = null, resource3 = null;
ExpectedException priorE = null;
try {
resource1 = ...; resource2 = ...; resource3 = ...; // Acquisition may
throw ExpectedException
..do..stuff.. // May throw ExpectedException
} catch (ExpectedException e) {
priorE = e;
} finally {
IOUtils.closeSafely(priorE, resource1, resource2, resource3);
}
The above code is identical to the Java7 try-with-resources (100% identical
behaviour, if Java7 is detected it will also log suppressed exceptions to
the priorE stack trace). It's just a few lines more code.
-----
Uwe Schindler
H.-H.-Meier-Allee 63, D-28213 Bremen
http://www.thetaphi.de
eMail: uwe@thetaphi.de
> -----Original Message-----
> From: Michael McCandless [mailto:lucene@mikemccandless.com]
> Sent: Thursday, September 01, 2011 12:18 PM
> To: user@tika.apache.org
> Subject: Re: Tika leaves files open
>
> On Thu, Sep 1, 2011 at 5:56 AM, Jukka Zitting <ju...@gmail.com>
> wrote:
> > Hi,
> >
> > On Thu, Sep 1, 2011 at 11:36 AM, Michael McCandless
> > <lu...@mikemccandless.com> wrote:
> >> While rare for IS.close() to throw an exception, if it does, it's
> >> quite awful because it masks the original exception. It seems best
> >> to be defensive?
> >
> > With closeQuietly() you'd really be throwing information out in case
> > where close() fails when no other exception has been thrown. Instead
> > of one exception masking another, you'd have no exceptions masking
> > one!
>
> Duh, you're right: we don't want to use closeQuietly if there was no
exception.
>
> For Lucene we do this:
>
> // open something
> boolean success = false;
> try {
> // do something
> success = true;
> } finally {
> if (!success) {
> closeQuietly();
> } else {
> closeNormally();
> }
> }
>
> This gets cleaner with Java 7 but it's some ways away before Tika can
require
> Java 7...
>
> Mike
Re: Tika leaves files open
Posted by Michael McCandless <lu...@mikemccandless.com>.
On Thu, Sep 1, 2011 at 5:56 AM, Jukka Zitting <ju...@gmail.com> wrote:
> Hi,
>
> On Thu, Sep 1, 2011 at 11:36 AM, Michael McCandless
> <lu...@mikemccandless.com> wrote:
>> While rare for IS.close() to throw an exception, if it does, it's
>> quite awful because it masks the original exception. It seems best to
>> be defensive?
>
> With closeQuietly() you'd really be throwing information out in case
> where close() fails when no other exception has been thrown. Instead
> of one exception masking another, you'd have no exceptions masking
> one!
Duh, you're right: we don't want to use closeQuietly if there was no exception.
For Lucene we do this:
// open something
boolean success = false;
try {
// do something
success = true;
} finally {
if (!success) {
closeQuietly();
} else {
closeNormally();
}
}
This gets cleaner with Java 7 but it's some ways away before Tika can
require Java 7...
Mike
Re: Tika leaves files open
Posted by Jukka Zitting <ju...@gmail.com>.
Hi,
On Thu, Sep 1, 2011 at 11:36 AM, Michael McCandless
<lu...@mikemccandless.com> wrote:
> While rare for IS.close() to throw an exception, if it does, it's
> quite awful because it masks the original exception. It seems best to
> be defensive?
With closeQuietly() you'd really be throwing information out in case
where close() fails when no other exception has been thrown. Instead
of one exception masking another, you'd have no exceptions masking
one!
BR,
Jukka Zitting