RFR(S) 8187040: ThreadCritical crashes on Solaris if used between os::init and os::init_2

[David Holmes]

On 2/09/2017 3:31 AM, Thomas Stüfe wrote:
> Hi Mikael,
> 
> (I never understood why we cannot just use pthread mutexes on Solaris. Why
> all this dynamic loading magic, are pthread functions not available on all
> Solaris versions?)

Depends how far back you want to go with "all" :) This is obviously 
strongly historical. We have three potential sync API's on Solaris 
(pthreads, UI threads and kernel LWPs). LWP sync tended to perform 
better** - and that's what we still use. We had a project a few years 
back to convert from UI threads to pthreads, but it was too big too 
manage at the time and was shelved.

** Unfortunately I have no idea how this was actually measured, so can't 
say whether this is still the case.

Cheers,
David

> Small nit, instead of adding a new variable _synchronization_initialized,
> how about _mutex_lock != NULL (in ThreadCritical()) and _mutex_unlock !=
> NULL (in ~ThreadCritical())?
> 
> I am okay with the removal of ::release(). Even if it were used, it is
> really safer to let the mutex live until process end.
> 
> I leave it up to you if you take my suggestion above. The patch is fine for
> me in the current form.
> 
> Kind Regards, Thomas
> 
> 
> On Fri, Sep 1, 2017 at 5:23 PM, Mikael Gerdin <mikael.gerdin at oracle.com>
> 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
>>