Le 11/08/2010 22:53, Mandy Chung a écrit :
On 08/11/10 12:35, Brian Goetz wrote:
This code has thread-safety issues. There is no happens-before
between the writes to suppressedExceptions (and its referenced state)
and the places where it is iterated (e.g., line 576).
You could plug this hole by replacing the idiom at line 576 (and
similar) with
for (Throwable se : getSuppressedExceptions()).
Good point. I fixed getSuppressedExceptions() be a synchronized
method but missed the unsynchronized reads.
This would be needed for all unsynchronized reads of
suppressedException. You could get away without it at the null check
if you really were trying to squeeze some performance out and you
didn't mind occasionally missing writes from other threads.
The updated webrev is at:
http://cr.openjdk.java.net/~mchung/6973831/webrev.01/
I leave the unsynchronized read in the readObject() method as the
Throwable object is still being deserialized and the
suppressedExceptions is going to be replaced with a new list.
Thanks
Mandy
Brian, Mandy,
It seems this is not the sole thread-safety issue,
access to fields 'cause' or 'stackTrace' aren't thread safe too and
detailMessage is not final.
Rémi
On Aug 11, 2010, at 3:09 PM, Mandy Chung wrote:
Fix for 6973831: NPE when printing stack trace of OOME
Webrev at:
http://cr.openjdk.java.net/~mchung/6973831/webrev.00/
The VM preallocates OutOfMemoryError objects to improve OOM
diagnosability. The constructor is not invoked since the OOME object
allocation is done at VM initialization time and thus the new
suppressedExceptions field is null which causes NPE when
Throwable.printStackTrace() method is called on the VM preallocated
OOM object.
The fix is to initialize the suppressedExceptions field to null
instead of Collections.emptyList(). An alternative could be to
initialize the suppressedException field in the VM after VM
initialization is finished. It doesn't handle the case when any new
field initialized to be non-null is added in the OOM object in the
future and it would require a VM synchronized change. The libraries
fix is simpler and preferred.
Thanks
Mandy
