[9] RFR (XS): 8138922: StubCodeDesc constructor publishes partially-constructed objects on StubCodeDesc::_list

[Vladimir Ivanov]

Coleen,




On 2/12/16 4:28 PM, Coleen Phillimore wrote:
>
> http://cr.openjdk.java.net/~vlivanov/8138922/webrev.01/src/share/vm/runtime/thread.cpp.udiff.html
>
>
> This has a collision with
>
>   RFR: 8148630: Convert TraceStartupTime to Unified Logging
Removed. I asked Rachel to cover java.lang.invoke case.

>
> http://cr.openjdk.java.net/~vlivanov/8138922/webrev.01/src/share/vm/code/codeBlob.cpp.udiff.html
>
>
> The 'new' operators already call vm_exit_out_of_memory rather than
> returning null. MethodHandlesAdapterBlob may already do this.
I don't see that happening in BufferBlob::operator new (which is called 
in ). It allocates right in the code cache and returns NULL if 
allocation fails.

I replicate the code from other stub generators, e.g.:
void StubRoutines::initialize2() {
...
     _code2 = BufferBlob::create("StubRoutines (2)", code_size2);
     if (_code2 == NULL) {
       vm_exit_out_of_memory(code_size2, OOM_MALLOC_ERROR, "CodeCache: 
no room for StubRoutines (2)");
     }

Or do you suggest to add MethodHandlesAdapterBlob::operator new and move 
the check there?

Updated webrev:
   http://cr.openjdk.java.net/~vlivanov/8138922/webrev.02

Had to move StubCodeDesc::freeze() call later in the init sequence: JFR 
also allocates some stubs.

Best regards,
Vladimir Ivanov

>
> Coleen
>
> On 2/12/16 8:13 AM, Vladimir Ivanov wrote:
>> Vladimir, David, Andrew, thanks again for the feedback.
>>
>> Updated version:
>>   http://cr.openjdk.java.net/~vlivanov/8138922/webrev.01
>>
>> I moved method handle adapters generation to VM init phase and added
>> verification logic to ensure there are no modifications to the
>> StubCodeDesc::_list after that.
>>
>> Also, slightly refactored java.lang.invoke initialization logic.
>>
>> Best regards,
>> Vladimir Ivanov
>>
>> On 2/10/16 9:13 PM, Vladimir Ivanov wrote:
>>> http://cr.openjdk.java.net/~vlivanov/8138922/webrev.00
>>> https://bugs.openjdk.java.net/browse/JDK-8138922
>>>
>>> StubCodeDesc keeps a list of all descriptors rooted at
>>> StubCodeDesc::_list by placing newly instantiated objects there at the
>>> end of the constructor. Unfortunately, it doesn't guarantee that only
>>> fully-constructed objects are visible, because compiler (or HW) can
>>> reorder the stores.
>>>
>>> Since method handle adapters are generated on demand when j.l.i
>>> framework is initialized, it's possible there are readers iterating over
>>> the list at the moment. It's not a problem per se until everybody sees a
>>> consistent view of the list.
>>>
>>> The fix is to insert a StoreStore barrier before registering an object
>>> on the list.
>>>
>>> (I also considered moving MH adapter allocation to VM initialization
>>> phase before anybody reads the list, but it's non-trivial since
>>> MethodHandles::generate_adapters() has a number of implicit
>>> dependencies.)
>>>
>>> Testing: manual (verified StubCodeMark assembly), JPRT
>>>
>>> Thanks!
>>>
>>> Best regards,
>>> Vladimir Ivanov
>