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