<html>
<head>
<meta http-equiv="content-type" content="text/html; charset=utf-8">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<p>Forward to runtime...<br>
</p>
<div class="moz-forward-container">This bug is a race condition
between allocating a java.lang.Class instance and concurrent GC. <br>
<a class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8158946">https://bugs.openjdk.java.net/browse/JDK-8158946</a><br>
<br>
There is an additional issue related to missing memory barriers in
storing and loading an array's length or a java.lang.Class'
oop_size field relative to the object's klass field, but that is
handled by "<a class="issue-link" data-issue-key="JDK-8160369"
href="https://bugs.openjdk.java.net/browse/JDK-8160369"
id="key-val" rel="4890629">JDK-8160369</a> Memory fences needed
around setting and reading object lengths."
<blockquote>Context:<br>
<br>
"As Kim mentioned, the new version sets the object size field of
a java.lang.Class object before it sets the object's header, so
GC can reliably get the size of any object with it's header set.
<br>
<br>
This fix works by adding a CollectedHeap::class_allocate()
method and associated helpers. These are implemented in
CollectedHeap.inline.hpp only because they share so much
structure and code with CollectedHeap::obj_allocate() that I
thought it best to keep them together (even though there is no
performance reason to have the new code inlined).
"<br>
</blockquote>
- Derek<br>
<br>
-------- Forwarded Message --------
<table class="moz-email-headers-table" border="0" cellpadding="0"
cellspacing="0">
<tbody>
<tr>
<th align="RIGHT" nowrap="nowrap" valign="BASELINE">Subject:
</th>
<td>Re: [RESUMED] RFR: 8158946 - btree009 fails with
assert(s > 0) failed: Bad size calculated</td>
</tr>
<tr>
<th align="RIGHT" nowrap="nowrap" valign="BASELINE">Date: </th>
<td>Tue, 28 Jun 2016 12:37:23 -0400</td>
</tr>
<tr>
<th align="RIGHT" nowrap="nowrap" valign="BASELINE">From: </th>
<td>Derek White <a class="moz-txt-link-rfc2396E" href="mailto:derek.white@oracle.com"><derek.white@oracle.com></a></td>
</tr>
<tr>
<th align="RIGHT" nowrap="nowrap" valign="BASELINE">Organization:
</th>
<td>Oracle</td>
</tr>
<tr>
<th align="RIGHT" nowrap="nowrap" valign="BASELINE">To: </th>
<td>Thomas Schatzl <a class="moz-txt-link-rfc2396E" href="mailto:thomas.schatzl@oracle.com"><thomas.schatzl@oracle.com></a>, Kim
Barrett <a class="moz-txt-link-rfc2396E" href="mailto:kim.barrett@oracle.com"><kim.barrett@oracle.com></a></td>
</tr>
<tr>
<th align="RIGHT" nowrap="nowrap" valign="BASELINE">CC: </th>
<td><a class="moz-txt-link-abbreviated" href="mailto:hotspot-gc-dev@openjdk.java.net">hotspot-gc-dev@openjdk.java.net</a></td>
</tr>
</tbody>
</table>
<br>
<br>
<pre>Hi Thomas,
New webrev based on your suggestions:
Webrev: <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~drwhite/8158946/webrev.03/">http://cr.openjdk.java.net/~drwhite/8158946/webrev.03/</a>
Incremental: <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~drwhite/8158946/webrev.02.vs.03">http://cr.openjdk.java.net/~drwhite/8158946/webrev.02.vs.03</a>
jprt in progress....
Misc comments below...
- Derek
On 6/28/16 9:09 AM, Thomas Schatzl wrote:
> Hi,
>
> On Mon, 2016-06-27 at 10:10 -0400, Derek White wrote:
>> I'd like to split out the memory fence issue from the race fixed by
>> this webrev. I think the fence issue may require more performance
>> testing and several attempts to get something satisfactory.
>>
>> New bug created:
>> JDK-8160369 Memory fences needed around setting and reading
>> object lengths.
>>
>> How do reviewers feel about this patch to fix the initial race
>> condition?
> looking at the 02 webrev:
>
> - <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~drwhite/8158946/webrev.02/src/share/vm/gc">http://cr.openjdk.java.net/~drwhite/8158946/webrev.02/src/share/vm/gc</a>
> /shared/collectedHeap.inline.hpp.frames.html
> 105 // set the j.l.Class instance's oop_size field BEFORE setting the
> header:
>
> I would like to have the "why" answered here in this comment and not a
> repetition of the code. I think something like: "Concurrent readers
> expect that the size is set before the klass pointer."
>
> Maybe the comment in lines 226/227 are more appropriate here?
>
> - the "obj" parameter is cast to an oop four times in
> CollectedHeap::post_allocation_setup_class. Could you add a local
> variable?
OK, I cleaned this up by mirroring post_allocation_setup_array().
> - <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~drwhite/8158946/webrev.02/src/share/vm/oo">http://cr.openjdk.java.net/~drwhite/8158946/webrev.02/src/share/vm/oo</a>
> ps/instanceMirrorKlass.cpp.frames.html
>
> Not sure if repeating the exit condition in line 59 makes sense.
OK.
> - <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~drwhite/8158946/webrev.02/src/share/vm/oo">http://cr.openjdk.java.net/~drwhite/8158946/webrev.02/src/share/vm/oo</a>
> ps/oop.inline.hpp.frames.html
>
> Maybe fix the comment in 261 to a proper sentence. (And possibly 262,
> like "Oop size must be larger than zero but is %d")
OK
Thanks for the suggestions!
- Derek
</pre>
</div>
</body>
</html>