RFR(S) 8187040: ThreadCritical crashes on Solaris if used between os::init and os::init_2
Mikael Gerdin
mikael.gerdin at oracle.com
Mon Sep 4 07:21:11 UTC 2017
Hi David,
On 2017-09-04 02:57, David Holmes wrote:
> Hi Mikael,
>
> This cleanup looks good. Obviously things got out of sync on solaris
> when we've split os::init into pieces in the past. And this solaris-only
> requirement should never have leaked through to the other ports. So good
> to see the cleanup.
Thanks for the quick review, David.
>
> As Thomas notes there may be something that can be implicitly checked
> instead of adding the new field, but that is a minor concern.
The reason I added the new field was to make it clear to the casual
reader of os::Solaris::synchronization_init() that other parts of the
code may be depending on the initialization.
If I just added accessors for the _mutex_lock and _mutex_unlock function
pointers I think that would be less clear.
I'm not strongly in favor of any approach, let's see what Thomas has to say.
/Mikael
>
> Thanks,
> David
>
> On 2/09/2017 1:23 AM, Mikael Gerdin wrote:
>> Hi,
>>
>> Please review this small fix to ThreadCritical.
>> When working on a piece of code which allocates memory early on I
>> noticed that it crashed if I enabled NMT.
>> The reason is that NMT uses ThreadCritical and os::Solaris sets the
>> ThreadCritical::_initialized flag before it actually sets up the
>> function pointers the flag is supposed to guard.
>> os::Solaris::_mutex_lock is not initialized until the init_2 phase
>> (after command line flag parsing).
>>
>> My suggested fix is to replace the current short-circuit of
>> ThreadCritical with a flag set when the Solaris mutex code is
>> initialized and thereby getting rid of the initialize function on all
>> platforms.
>> Additionally, ThreadCritical::release is unreachable code and from my
>> research has never actually been called, we might as well get rid of it.
>>
>> Webrev: http://cr.openjdk.java.net/~mgerdin/8187040/webrev.0/
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8187040
>> Testing: JPRT
>>
>> Thanks
>> /Mikael
More information about the hotspot-runtime-dev
mailing list