RFR(S) 8187040: ThreadCritical crashes on Solaris if used between os::init and os::init_2
mikael.gerdin at oracle.com
Mon Sep 4 07:26:03 UTC 2017
On 2017-09-01 19:31, 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?)
Beats me, but I suspect that David is right. At some point someone
thought one was better and now nobody remembers how and nobody wants to
put in the performance work to determine if it actually makes a difference.
> Small nit, instead of adding a new variable
> _synchronization_initialized, how about _mutex_lock != NULL (in
> ThreadCritical()) and _mutex_unlock != NULL (in ~ThreadCritical())?
I didn't want to expose the function pointers through accessors in
os::Solaris and I'm worried that if we check a different thing in the
lock versus unlock paths we can end up with a ThreadCritical which tries
to unlock a lock which was never locked (because the TC was created
before _mutex_lock was set).
Also, I think it's clearer to the reader of
os::Solaris::synchronization_init that the "initialization completed"
state is exposed to an external caller.
> 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.
Thanks for the review, Thomas.
> 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:
> 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
More information about the hotspot-runtime-dev