Maybe I'm wrong but you can reuse the same clone.
I mean, it has to have a different identity from UNASSIGNED_STACK but
it can be the same for all clones.
Rémi
On 04/08/2011 08:27 PM, Joe Darcy wrote:
Updated webrev described below...
Joe Darcy wrote:
Joe Darcy wrote:
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
PS A little more detail here, exception objects can be created via
deserialization as well as by a constructor. As currently written,
the writeObject method uses EMPTY_STACK for both a zero-length
incoming stack and a null stackTrace pointer (which can result from
an throwable object serialized from a JDK release before the stack
trace facility was added). The native fillInStackTrace method writes
a null into the stackTrace field to indicate it is no longer valid;
this goes against the new protocol and the Java-level of
fillInStackTraces needs to write a non-null "dirty" value into the
field to signal to getOurStackTrace that the stack trace array needs
to be recomputed if requested. Therefore, if fillInStackTrace were
called on one of these deserialized objects to logically replace its
stack trace, using EMPTY_STACK would not allow the getOutStackTrace
code to know that the stack trace had logically been replaced by a
new value in backtrace since EMPY_STACK could result from serialization.
The serialization code could use EMPTY_STACK.clone() instead of
EMPTY_STACK, which would break the object equality being tested in
getOurStackTrace. I'll consider making this change in the
Throwable.readObject method.
Cheers,
-Joe
I've put the updated webrev up at
http://cr.openjdk.java.net/~darcy/6998871.4/
Diff of Throwable compared to .3 version is below. EMPTY_STACK has
been renamed to UNASSIGNED_STACK and that value (or a clone if it) is
used as the sentential and for zero-length arrays. The distinct
FILLED_IN_STACK value has been removed. The protocol around updating
the fields is better documented. In other files, ArithmeticException,
NullPointerException, and OutOfMemoryError are updated to state the VM
may created them with immutable stack too.
Thanks,
-Joe
157,163d156
< * A value indicating that the logical stack trace has been
< * populated into the backtrace field.
< */
< private static final StackTraceElement[] FILLED_IN_STACK =
< new StackTraceElement[] {new StackTraceElement("FILLED_IN",
"STACK", null, -1)};
<
< /**
166c159
< private static final StackTraceElement[] EMPTY_STACK = new
StackTraceElement[0];
---
> private static final StackTraceElement[] UNASSIGNED_STACK = new
StackTraceElement[0];
211c204,205
< * #setStackTrace()} and {@link #fillInStackTrace} will be be
no-ops.
---
> * #setStackTrace(StackTraceElement[])} and {@link
> * #fillInStackTrace()} will be be no-ops.
216c210
< private StackTraceElement[] stackTrace = EMPTY_STACK;
---
> private StackTraceElement[] stackTrace = UNASSIGNED_STACK;
330c324,325
< * #fillInStackTrace()} and subsequent calls to {@code
---
> * #fillInStackTrace()}, a {@code null} will be written to the
> * {@code stackTrace} field, and subsequent calls to {@code
339,342c334,339
< * conditions under which suppression is disabled. Disabling of
< * suppression should only occur in exceptional circumstances
< * where special requirements exist, such as a virtual machine
< * reusing exception objects under low-memory situations.
---
> * conditions under which suppression is disabled and document
> * conditions under which the stack trace is not writable.
> * Disabling of suppression should only occur in exceptional
> * circumstances where special requirements exist, such as a
> * virtual machine reusing exception objects under low-memory
> * situations.
770c767
< stackTrace = FILLED_IN_STACK;
---
> stackTrace = UNASSIGNED_STACK;
807c804
< if (stackTrace == FILLED_IN_STACK) {
---
> if (stackTrace == UNASSIGNED_STACK) {
813c810
< return EMPTY_STACK;
---
> return UNASSIGNED_STACK;
886,888c883,886
< * trace elements. A single-element stack trace whose entry is
< * equal to {@code new StackTraceElement("", "", null,
< * Integer.MIN_VALUE)} results in a {@code null} {@code
---
> * trace elements. A null stack trace in the serial form results
> * in a zero-length stack element array. A single-element stack
> * trace whose entry is equal to {@code new StackTraceElement("",
> * "", null, Integer.MIN_VALUE)} results in a {@code null} {@code
918c916,924
< // Check for the marker of an immutable stack trace
---
> /*
> * For zero-length stack traces, use a clone of
> * UNASSIGNED_STACK rather than UNASSIGNED_STACK itself to
> * allow identity comparison against UNASSIGNED_STACK in
> * getOurStackTrace. The identity of UNASSIGNED_STACK in
> * stackTrace indicates to the getOurStackTrace method that
> * the stackTrace needs to be constructed from the information
> * in backtrace.
> */
920d925
< // Share zero-length stack traces
922c927
< stackTrace = EMPTY_STACK;
---
> stackTrace = UNASSIGNED_STACK.clone();
923a929
> // Check for the marker of an immutable
stack trace
937c943
< stackTrace = EMPTY_STACK;
---
> stackTrace = UNASSIGNED_STACK.clone();
976,977c982,983
< * {@linkplain #Throwable(String, Throwable, boolean) via a
< * constructor}. When suppression is disabled, this method does
---
> * {@linkplain #Throwable(String, Throwable, boolean, boolean) via
> * a constructor}. When suppression is disabled, this method does
1043,1044c1049,1050
< * #Throwable(String, Throwable, boolean) suppression is disabled},
< * an empty array is returned.
---
> * #Throwable(String, Throwable, boolean, boolean) suppression is
> * disabled}, an empty array is returned.
