[PATCH] JDK-6937053: RMI unmarshalling errors in ClientNotifForwarder cause silent failure

Jaroslav Bachorik jaroslav.bachorik at oracle.com
Thu Oct 25 03:58:51 PDT 2012


Thanks, addressing the comments ...

On 10/24/2012 05:43 PM, Eamonn McManus wrote:
> The fix looks good, but I would adjust a couple of small things. First, you
> can improve the existing and new code with multi-catch. Second, the message
> in the if (notFoundCount > 0) block should be adjusted to take into account
I was not really sure about the wording of the message - I've added the
part warning about incompatibility. But if you have something better in
mind I would happily use it.

> the new cases. Finally, hardwiring port 12345 into the test makes it a bit
> fragile. It would be better for the server to listen on any available port
> and (for example) write which port it is, or what its JMXServiceURL is, on
> its stdout. The shell script can then read that and launch the client with
> it. That would also get rid of the arbitrary sleep 3.
Done and done.

Webrev at http://cr.openjdk.java.net/~jbachorik/JDK-6937053/webrev.02/

-JB-

> 
> Éamonn
> 
> 
> 2012/10/24 Jaroslav Bachorik <jaroslav.bachorik at oracle.com>
> 
>> Updated webrev at
>> http://cr.openjdk.java.net/~jbachorik/JDK-6937053/webrev.01/ - removed a
>> dangling debug output.
>>
>> -JB-
>>
>> On 10/24/2012 04:03 PM, Jaroslav Bachorik wrote:
>>> I am looking for review and a sponsor.
>>>
>>> Webrev available at
>>> http://cr.openjdk.java.net/~jbachorik/JDK-6937053/webrev.00/
>>>
>>> The RMI marshalling process may throw java.rmi.UnmarshallException eg.
>>> in cases of incompatible changes in enums. The bad thing is that
>>> ClientNotifForwarder chooses to silently die instead of reporting the
>>> problem.
>>>
>>> The fix consists of adding support for handling
>>> java.rmi.UnmarshallException the same way as
>>> java.io.NotSerializableException and appropriate changes in the javadoc.
>>>
>>> Thanks,
>>>
>>> -JB-
>>>
>>
>>
> 



More information about the serviceability-dev mailing list