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

[Thomas Stüfe]

Hi David,

On Mon, Sep 4, 2017 at 3:04 AM, David Holmes <david.holmes at oracle.com>
wrote:

> 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.
>
>
Thanks for that history lesson!

One thing is to use prefer lwp over pthread, but another is to link all of
that dynamically. I wondered whether we could not just call
pthread_mutex_link() directly, without dlsyming it first. This would be a
small thing to cleanup in comparison to switching to pthread APIs
completely.

Cheers, Thomas


> ** 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
>>>
>>>