On 17/03/2010, Mark Thomas <ma...@apache.org> wrote:
> On 17/03/2010 22:04, sebb wrote:
>
> > On 17/03/2010, Mark Thomas<ma...@apache.org> wrote:
> >
> > > One of the POOL test cases is failing -
> > > TestSoftRefOutOfMemory.testOutOfMemoryError()
> > >
> > > I can't see any recent code changes that may have caused this and I
> think
> > > the recent removal of the testAll code is what has exposed this. On the
> > > basis that always catching Throwable is bad, but there are many cases
> POOL
> > > does need to catch, what do folks here think to the liberal application
> of
> > > the following anywhere POOL catches Throwable?
> > >
> >
> > ThreadDeath should never be ignored either.
> >
> Yep.
>
>
> > Is it necessary to swallow all Throwables apart from VME?
> >
> That depends on your definition of necessary. I'd like to keep the list of
> exceptions as small as possible.
>
> On reflection, I think I'll put this in a utility method so the list of
> exceptions only has to be maintained in a single place. That will help keep
> the code clean even if we end up with a long list of Throwables we don't
> want to swallow.
Sounds good.
Seems to me it would be useful to be able to record or log the
swallowed Throwables.
If the Throwable is caused by a user error, then swallowing it means
that the error is likely to be much harder to diagnose and fix.
>
> > Maybe should rethrow some other classes of Throwable as well.
> >
> > Is it known which Throwables Pool does need to catch?
> >
> No fixed list. I'd say as many as possible.
>
> Mark
>
>
>
>
> >
> >
> > > Index:
> > >
> src/java/org/apache/commons/pool/impl/SoftReferenceObjectPool.java
> > >
> ===================================================================
> > > ---
> > >
> src/java/org/apache/commons/pool/impl/SoftReferenceObjectPool.java
> > > (revision 924253)
> > > +++
> > >
> src/java/org/apache/commons/pool/impl/SoftReferenceObjectPool.java
> > > (working copy)
> > > @@ -106,10 +106,18 @@
> > > throw new Exception("ValidateObject failed");
> > > }
> > > } catch (Throwable t) {
> > > + if (t instanceof VirtualMachineError) {
> > > + // Always throw VM errors immediately
> > > + throw (VirtualMachineError) t;
> > > + }
> > > try {
> > > _factory.destroyObject(obj);
> > > } catch (Throwable t2) {
> > > - // swallowed
> > > + if (t2 instanceof VirtualMachineError) {
> > > + // Always throw VM errors immediately
> > > + throw (VirtualMachineError) t2;
> > > + }
> > > + // Otherwise swallowed
> > > } finally {
> > > obj = null;
> > > }
> > >
> > > This fixes the current test failures but really should be applied to
> all
> > > places where Throwable is caught.
> > >
> > > Mark
> > >
> > >
> > >
> > >
> ---------------------------------------------------------------------
> > > To unsubscribe, e-mail:
> dev-unsubscr...@commons.apache.org
> > > For additional commands, e-mail: dev-h...@commons.apache.org
> > >
> > >
> > >
> >
> >
> ---------------------------------------------------------------------
> > To unsubscribe, e-mail:
> dev-unsubscr...@commons.apache.org
> > For additional commands, e-mail: dev-h...@commons.apache.org
> >
> >
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
