Code review request for 6998871 "Support making the Throwable.stackTrace field immutable"

[Joe Darcy]

Hi Rémi.

Rémi Forax wrote:
> On 04/07/2011 08:29 AM, Joe Darcy wrote:
>> Hello.
>>
>> Returning to some earlier work, I've developed a proposed fix for
>>
>>    6998871 "Support making the Throwable.stackTrace field immutable"
>>    http://cr.openjdk.java.net/~darcy/6998871.2/
>>
>> One constructor of Throwable now takes an additional boolean argument 
>> to make the stack trace information immutable.  Analogous 
>> constructors are added to Exception, RuntimeException, and Error.
>>
>> Mandy and David have already reviewed the change; I'm interested in 
>> getting additional feedback on the design of the API.
>>
>> Cheers,
>>
>> -Joe
>
> Joe,
> I don't think you need the sentinel in the serialized form,
> you have only two states: an immutable stacktrace (stacktrace == null) or
> a stacktrace. I think it's better to don't serialize the field 
> "stacktrace" if
> the stacktrace is immutable.
>
> Also, FILLED_IN_STACK is not necessary, you can use EMPTY_STACKinstead,
> or if you find it hard to understand, at least FILLED_IN_STACK should 
> be an empty array.
>
> Rémi
>

Here is an explanation and rationale of the protocol in more detail.  As 
far as serialization goes, there are three cases of interest where "new" 
means JDK 7 and "old" means any release prior to JDK 7

1) Object serialized on *new* platform has same semantics after being 
deserialized on *new* platform.

2) Object serialized on *old* platform has same semantics after being 
deserialized on *new* platform.

3) Object serialized on *new* platform has reasonable semantics after 
being deserialized on *old* platform.

(And of course when both source and destination platforms are old, JDK 7 
isn't involved.)

For point 1), a logically equivalent object should result, which in this 
case means the cause, stack trace, stack trace, message, etc. should be 
the same on the old and new objects.

For 2), if an exception object serialized prior to the stack trace 
facility being added was deserialized in JDK 7, that is logically 
equivalent to there being not stack trace information, so an empty stack 
is a better result than an immutable stack.  I've updated the readObject 
method accordingly.

For 3), since null was not a valid value for stackTrace in the past, 
some other sentinel object should be written in out in the serial form 
to denote such a value, which is the role played by STACK_TRACE_SENTINEL 
as used in the writeObject method.

A marker value other than EMPTY_STACK is needed for the following reason 
since the stack trace information is spread amongst two fields, 
backtrace and stackTrace.  The transient backtrace field pre-dates the 
programmatic stack facility being added; that facility uses the 
stackTrace field.  If fillInStackTrace is called *after* a call to 
setStackTrace, the real stack information is held in the backtrace field 
as written by fillInStackTrace.  The FILLED_IN_STACK value alerts the 
getourstacktrace method to this situation.

Revised webrev:

    http://cr.openjdk.java.net/~darcy/6998871.3/

Thanks,

-Joe