commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Thomas <ma...@apache.org>
Subject Re: [POOL] Test failures
Date Wed, 17 Mar 2010 22:21:04 GMT
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?
>
> 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.

> 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


Mime
View raw message