Hi,

Le 08/03/2011 23:51, Jörg Schaible a écrit :
> sebb wrote:
> 
>> On 8 March 2011 20:17, Luc Maisonobe <luc.maison...@free.fr> wrote:
>>> Le 07/03/2011 11:37, er...@apache.org a écrit :
>>>> Author: erans
>>>> Date: Mon Mar  7 10:37:52 2011
>>>> New Revision: 1078734
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1078734&view=rev
>>>> Log:
>>>> MATH-542
>>>> "MathRuntimeException" provides two ways for enhancing the information
>>>> content: one for localized messages and one for storing "context"
>>>> objects. The additional methods have been added to "MathThrowable".
>>>> Consequently, dummy implementations were needed in the old
>>>> "MathRuntimeException" and "MathException".
>>>> Some parts of the tests for old exceptions were removed as they used
>>>> methods that were deleted. A call to "MathUserException" in
>>>> "AbstractContinuousDistribution" was simplified for the same reason.
>>>> "MathUtils" still contained "String" (instead of "Localizable") as
>>>> argument to the constructor of a "MathArithmeticException".
>>>> I don't know why a test in "DummyStepInterpolatorTest" stopped working.
>>>> It is now commented out; please have a look.
>>>
>>> [snip]
>>>
>>>
>>>> Modified:
>>>>
> commons/proper/math/trunk/src/test/java/org/apache/commons/math/ode/sampling/DummyStepInterpolatorTest.java
>>>> URL:
>>>>
> http://svn.apache.org/viewvc/commons/proper/math/trunk/src/test/java/org/apache/commons/math/ode/sampling/DummyStepInterpolatorTest.java?rev=1078734&r1=1078733&r2=1078734&view=diff
>>>>
> ==============================================================================
>>>> ---
>>>>
> commons/proper/math/trunk/src/test/java/org/apache/commons/math/ode/sampling/DummyStepInterpolatorTest.java
>>>> (original) +++
>>>>
> commons/proper/math/trunk/src/test/java/org/apache/commons/math/ode/sampling/DummyStepInterpolatorTest.java
>>>> Mon Mar  7 10:37:52 2011 @@ -123,7 +123,9 @@ public class
>>>> DummyStepInterpolatorTest { fail("an exception should have been
>>>> thrown"); } catch (IOException ioe) { // expected behavior -       
>>>> assertEquals(0, ioe.getMessage().length()); +        // XXX Why was the
>>>> message supposed to be empty? +        // With the current code it is
>>>> "org.apache.commons.math.util.Pair". +        // assertEquals(0,
>>>> ioe.getMessage().length());
>>>
>>> The problem here is that the exception thrown which is an IOException
>>> built from the empty string in the BadStepInterpolator below is not the
>>> IOException that is caught here. In fact the IOException caught is
>>> really a java.io.NotSerializableException which is built with the
>>> message "org.apache.commons.math.util.Pair" automatically by the
>>> serialization process in the JVM (at least for Oracle implementation).
>>>
>>> All exceptions must be serializable (this comes from java.lang.Throwable
>>> implementing the Serializable interface). This was also one reason why
>>> our Localizable interface had to extends Serializable, so it could be
>>> used as a field in our exceptions.
>>>
>>> This change added a List<Pair<Localizable, Object[]>> field in
>>> MathRuntimeException. Pair is not serializable. Adding Serializable to
>>> the implemented interfaces in Pair does solve the problem (if also the
>>> change below is reverted, of course) but I don't think it would be wise
>>> unless we also enforce the two generic parameters of Pair to be
>>> serializable too. Perhaps we should use a Map<Localizable, Object[]>
>>> instead ?
>>>
>>> The problem also makes me think that we already had a similar bug in
>>> another part of the exceptions for a long time : the Object or Object[]
>>> parameters of the exceptions are stored and may not be Serializable too.
>>> This was never a problem either because the parameters are often simple
>>> primitive ones (int, double ...) or arrays so they were serializable.
>>> Perhaps we should change the signature from Object and Object[] to
>>> Serializable and Serializable[] ?
>>>
>>
>> Or make the fields transient?
>>
>> That would perhaps cause problems if the Exceptions were ever
>> serialised, but can that happen?
> 
> Yes, it's a typical JEE scenario and I cannot say how often I cursed Sun for 
> stuffing an unserializable object into NamingException.setResolvedObject 
> within their LDAP implementation ...
> 
> Actually the MathRE should contain a writeObject implementation that 
> replaces any non-serializable object in that array with some kind of 
> replacement (e.g. with its String representation). Otherwise the MathRE will 
> not reach its destination and all the localization was for nothing.

That's a good idea. We could even do that replacement directly at
construction and never store the Object themselves, regardless of their
status with respect to serialization.

Thanks Jörg

> 
> However, with a writeObject/readObject pair, such a field can be made 
> transparent, if the objects are put directly into the object streams.
> 
> Note, that anything of this requires a change of the serialVersionUID.
> 
> - Jörg
> 
> 
> ---------------------------------------------------------------------
> 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

Reply via email to