commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Phil Steitz <phil.ste...@gmail.com>
Subject Re: [POOL] Test failures
Date Thu, 18 Mar 2010 01:11:54 GMT
sebb wrote:
> On 17/03/2010, Mark Thomas <markt@apache.org> wrote:
>> On 17/03/2010 22:04, sebb wrote:
>>
>>> On 17/03/2010, Mark Thomas<markt@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?

OK with your suggestion.
>>>>
>>> ThreadDeath should never be ignored either.
>>>
>>  Yep.

+1
>>
>>
>>> 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.

+1
> 
> 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.

I agree with this.  My only concern is users who are relying on
current behavior.
> 
>>> 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-unsubscribe@commons.apache.org
>>>>  For additional commands, e-mail: dev-help@commons.apache.org
>>>>
>>>>
>>>>
>>>
>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail:
>> dev-unsubscribe@commons.apache.org
>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>
>>>
>>
>>
>>
>> ---------------------------------------------------------------------
>>  To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>  For additional commands, e-mail: dev-help@commons.apache.org
>>
>>
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message