Updated ARM Spec

[Joe Darcy]

Rémi Forax wrote:
> Le 23/08/2010 12:29, David Holmes a écrit :
>   
>> Rémi Forax said the following on 08/23/10 20:23:
>>     
>>> Le 23/08/2010 12:09, David Holmes a écrit :
>>>       
>>>> Rémi Forax said the following on 08/23/10 20:06:
>>>>         
>>>>> You can explicitly call setStackTrace() on a shared immutable 
>>>>> exception, hence there is a problem.
>>>>> addSupressedExceptions() will have the same issue.
>>>>>           
>>>> I stand corrected. The Java side of the code couldn't care less 
>>>> about the VM level backtrace - it just installs a Java array. :(
>>>>
>>>>         
>>>>> I think the fix is to:
>>>>> - silently discard the stack trace taken as argument of 
>>>>> setStackTrace() if the cause is 'null' (or not 'this')
>>>>> - silently don't register suppressed-exceptions if cause is 'null'
>>>>>           
>>>> I think the Java code should be able to recognize a Throwable 
>>>> instance that should not allow these things to be set, and simply 
>>>> ignore the request. However that is a change in the spec for 
>>>> setStackTrace.
>>>>         
>>> Yes, setStackTrace() can 'not succeed'.
>>> And the way to recognize such Throwable instance is to check is the 
>>> field 'cause' is null.
>>>       
>> Except if someone invokes setCause(null).
>>     
>
> setCause(null) should set the field cause to this.
> For Throwable(null) and Throwable(message, null), null should also be 
> escaped.
>
> The other solution is to use the field suppressedExceptions with the 
> invariant that if
> it's null, it's an immutable shared exception.
>
>   
>> By the way, probing further, the non-OOME shared pre-allocated 
>> instances do not guard themselves against fillInStackTrace being 
>> invoked upon them. That is arguably an additional bug. I say arguably 
>> simply because in the circumstances under which those other exceptions 
>> are thrown, having a misleading stacktrace is probably the least of 
>> the application's worries.
>>     
>
> yes, you right.
>
>   

To recap, Throwable objects have several pieces of mutable state:

* the cause; this is a write-at-most-once value that can be set set via 
a constructor, like Throwable(Throwable cause) or Throwable(String 
message, Throwable cause), or set after construction via the 
Throwable.initCause method.  (Setting the cause to null prevents future 
assignment.)

* the stack trace; set by fillInStackTrace() or 
setStackTrace(StackTraceElement[] stackTrace) -- there doesn't seem to 
be any API prohibition against setting the stack trace multiple times

* suppressed exceptions : new functionality added to support 
try-with-resources statements/ARM blocks.  Modified via calling 
addSuppressedException

Focusing just on suppressed exceptions, to support the JVM reusing 
exception objects, the Throwable API should have some idiom to indicate 
suppressed exception information should *not* be recorded. Logically 
this amount to having the list of exceptions be a zero-length list which 
just discards adds.

I'm hesitant to overload a null cause with discarding suppressed 
exceptions too.  Instead, I propose the following:

* a null value of the suppressedException field indicates 
addSuppressedException is a no-op.

* the suppressedException field is initialized to point to a sentinel 
list in Throwable, something non-null that can be used for == checks.  
With this protocol, the "raw" Throwable objects created by the JVM get 
the addSuppressedException is a no-op behavior for free.

* if the first call to addSuppressedException has this as an argument, 
the suppressedException field is null-ed; otherwise, a list is appended 
to as usual.

Comments?

-Joe