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

David Holmes david.holmes at oracle.com
Mon Sep 4 21:40:20 UTC 2017


On 4/09/2017 7:36 PM, Thomas Stüfe wrote:
> Hi David,
> 
> On Mon, Sep 4, 2017 at 3:04 AM, David Holmes <david.holmes at oracle.com 
> <mailto: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.

I agree - the dlsym'ing dates back to separate thread libraries, and is 
not needed. I also noticed we don't use the proper 
pthread_cond/mutex_init functions, but have a raw memset - obviously 
dealing with an early omission from the API! But I don't even know if 
switching to use pthreads sync even works these days - it is never 
tested AFAIK.

David

> 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 <mailto: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/
>             <http://cr.openjdk.java.net/~mgerdin/8187040/webrev.0/>
>             Bug: https://bugs.openjdk.java.net/browse/JDK-8187040
>             <https://bugs.openjdk.java.net/browse/JDK-8187040>
>             Testing: JPRT
> 
>             Thanks
>             /Mikael
> 
> 


More information about the hotspot-runtime-dev mailing list