openjdk9 on Freebsd

David Holmes david.holmes at oracle.com
Sun Mar 20 22:20:42 UTC 2016


Sorry I didn't seem this before my previous reply ...

On 21/03/2016 4:53 AM, Brian Gardner wrote:
> I looked into the origination of other calls to set_thread(NULL) in
> openjdk8.
>
>   * hotspot/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepThread.cpp:163:
>     ThreadLocalStorage::set_thread(NULL);
>   * hotspot/src/share/vm/gc_implementation/shared/concurrentGCThread.cpp:91:
>     ThreadLocalStorage::set_thread(NULL);
>   * hotspot/src/share/vm/runtime/thread.cpp:1351:
>     ThreadLocalStorage::set_thread(NULL);
>
> These are use cases where set_thread(NULL) was called roughly towards
> the end of the threads run() method.  Annotating them showed they’ve
> been around prior to being open sourced.  At some point in time there
> was an effort made to ensure set_thread(NULL) was always called on all
> threads, and there is a degradation in openjdk9 in these regards.

Any thread that terminates explicitly will do this as part of the Thread 
destructor. For other threads it didn't seem necessary given they would 
terminate only when the process terminated.

> If cleaning things up in the Thread destructor (Thread::~Thread) worked
> on all platforms, it would be a clean way to handle the cleanup.  But
> since this doesn’t get called on all platforms, this logic should have
> been moved to JavaThread::run().  WatcherThread::run initializes it’s
> TLS and cleans it up, why not have JavaThread::run() do the same?  I
> don’t like JavaThread::run leaving it up to all it’s implementations to
> clean themselves up.

This is not a platform specific issue. Thread::~Thread gets called, or 
not, for the same set of threads on all platforms. The VMThread, nor 
those other threads, is not a JavaThread so JavaThread::run does not 
come into it. And for JavaThreads we don't want to do this cleanup in 
JavaThread::run because we may still need to refer to the current 
thread, so it is done during Thread::~Thread when the thread has 
"terminated" as far as the VM is concerned.

The problem at hand only affects threads that never terminate/detach. 
They previously would call set_thread(NULL) (though why is unclear), but 
after my changes they don't. But that can be rectified and as I said I 
will file a bug to handle that.

Please note however I am heading out on vacation in two days so this is 
unlikely to get fixed until I return in a couple of weeks.

Thanks,
David

> Brian Gardner
>
>
>> On Mar 19, 2016, at 11:46 PM, Brian Gardner <openjdk at getsnappy.com
>> <mailto:openjdk at getsnappy.com>> wrote:
>>
>> Yes, I was referring to the call to ThreadLocalStorage::set_thread(NULL).
>>
>> The problem I'm trying to resolve are the pthread destructors being
>> called repeadly.  On linux this isn’t a problem because the destructor
>> is called 4 times then silently gives up.  On FreeBSD the destructor
>> is called 4 times then prints a warning to stderr, which is a problem,
>> although it is harmless.
>>
>> The message below from the original thread states there are three
>> scenarios threads fall into in regards to the initial commit. The
>> third scenario is the problem scenario I just mentioned and while it
>> is ok on Linux, it isn’t ok on Freebsd because of the warnings to stderr.
>>
>> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2014-February/010796.html
>> I didn’t articulate it very well but I was trying to say that calling
>> ThreadLocalStorage::set_thread(NULL) durning thread cleanup is the
>> proper way to prevent the destructor from being called repeatedly.  I
>> actually can’t think of an alternate way to do this.
>> In openjdk8 all threads clean themselves by calling
>> ThreadLocalStorage::set_thread(NULL). I’m going to do some more
>> research to locate the change sets for these calls. Perhaps they are
>> isolated to bsd-port branch. I’ll let everyone know what I find.
>>
>> Kind regards,
>> Brian Gardner
>>
>>
>>> On Mar 18, 2016, at 2:57 PM, David Holmes <david.holmes at oracle.com
>>> <mailto:david.holmes at oracle.com>> wrote:
>>>
>>> On 19/03/2016 2:58 AM, Brian Gardner wrote:
>>>> That explains the destructor.  Looking at the initial change set that
>>>> came out of this bug, we also see the first spot where we set the TLS
>>>> current thread is set to NULL
>>>> http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/469835cd5494
>>>
>>> Are you referring to:
>>>
>>> +  // Thread destructor usually does this.
>>> +  ThreadLocalStorage::set_thread(NULL);
>>>
>>> ? That's only there because the VMThread destructor is never called.
>>>
>>>> So I think it’s safe to say that setting TLS current thread to NULL is
>>>> the correct way to set the state to "destroying thread" and preventing
>>>> the destructor from looping indefinitely.
>>>
>>> I'm not sure exactly what you mean.
>>>
>>>>
>>>> Here is the relevant comment from Andreas:
>>>>
>>>> --------------------------------
>>>>
>>>> I did find a way to change the JVM to workaround this problem:
>>>> By creating a destructor for the thread pointer TLS we can restore the
>>>> value after pthread has set it to NULL.
>>>> Then when the native code destructor is run the thread pointer is still
>>>> intact.
>>>>
>>>> Restoring a value in a pthread TLS is explicitly supported according to
>>>> the man page for pthread_key_create, and it will call the destructor for
>>>> the restored value again.
>>>> One would have to keep some extra state to make sure the destructor is
>>>> only called twice, since a pthread implementation is allowed to call the
>>>> destructor infinite times as long as the value is restored.
>>>>
>>>> On my system pthread calls the destructor a maximum of four times, so
>>>> the attached JVM patch was sufficient as a proof of concept.
>>>>
>>>> ————————————————
>>>
>>> We implemented the basic patch, we don't do anything to ensure it was
>>> called at most twice. We expect all well behaving apps to detach
>>> threads from the JVM before they terminate.
>>>
>>> David
>>> -----
>>>
>>>>
>>>>> On Mar 17, 2016, at 9:02 PM, David Holmes <david.holmes at oracle.com
>>>>> <mailto:david.holmes at oracle.com>
>>>>> <mailto:david.holmes at oracle.com>> wrote:
>>>>>
>>>>> Thomas writes:
>>>>>> Hi Brian,
>>>>>>
>>>>>>
>>>>>>> The next patches where less straightforward.  When running java I was
>>>>>>> getting a ton of messages like:
>>>>>>> Thread 832744400 has exited with leftover thread-specific data
>>>>>>> after 4
>>>>>>> destructor iterations
>>>>>>> After doing a lot of digging and debugging on Linux, I found the
>>>>>>> code path
>>>>>>> for Linux was identical for Freebsd and the cleanup destructor
>>>>>>> was being
>>>>>>> executed 4 times just like Freebsd, the difference being that
>>>>>>> Freebsd would
>>>>>>> print out this benign warning while Linux would just ignore it.  The
>>>>>>> problem is that all threads that are created and initialize TLS
>>>>>>> current
>>>>>>> thread data, must clean them up by explicitly setting the TLS current
>>>>>>> thread to null.  I’ve come up with two approaches to accomplish this.
>>>>>>>
>>>>>>> clean up TLS current thread at end of ::run functions similar to how
>>>>>>> it's
>>>>>>> done in openjdk8.
>>>>>>>
>>>>>>> http://brian.timestudybuddy.com/webrev/hotspot__clear_thread_current/webrev/
>>>>>>> clear current thread before exiting java_start to avoid warnings from
>>>>>>> leftover pthread_setspecific data
>>>>>>>
>>>>>>> http://brian.timestudybuddy.com/webrev/hotspot__clear_thread_current_alt/webrev/
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>> I do not think this is a real leak. From what I remember of how
>>>>>> the glibc
>>>>>> implements TLS, setting the TLS slot value to NULL would not in itself
>>>>>> delete anything. In VM, this slot keeps the pointer to the current
>>>>>> Thread*,
>>>>>> which is correctly deleted at the end of the thread (void
>>>>>> JavaThread::thread_main_inner()).
>>>>>>
>>>>>> Digging further, I found the pthread key destructor
>>>>>> "restore_thread_pointer(void* p)" in threadLocalStorage_posix.cpp:
>>>>>>
>>>>>> // Restore the thread pointer if the destructor is called. This is in
>>>>>> case
>>>>>> // someone from JNI code sets up a destructor with pthread_key_create
>>>>>> to run
>>>>>> // detachCurrentThread on thread death. Unless we restore the thread
>>>>>> pointer we
>>>>>> // will hang or crash. When detachCurrentThread is called the key
>>>>>> will be
>>>>>> set
>>>>>> // to null and we will not be called again. If detachCurrentThread is
>>>>>> never
>>>>>> // called we could loop forever depending on the pthread
>>>>>> implementation.
>>>>>> extern "C" void restore_thread_pointer(void* p) {
>>>>>> ThreadLocalStorage::set_thread((Thread*) p);
>>>>>> }
>>>>>>
>>>>>> So, it seems we even reset deliberately the thread pointer to a
>>>>>> non-NULL
>>>>>> value. The comment claims that we reset the Thread* value in case
>>>>>> there is
>>>>>> another user-provided destructor which runs afterwards and which
>>>>>> does detachCurrentThread () which would require Thread::current() to
>>>>>> work.
>>>>>> But there a details I do not understand:
>>>>>>
>>>>>> - At this point, should the Thread* object not already be
>>>>>> deallocated, so
>>>>>> this would be a dangling pointer anyway?
>>>>>>
>>>>>> - Also, according to Posix, this is unspecified. Doc on
>>>>>> pthread_setspecific() states: "Calling pthread_setspecific() from a
>>>>>> thread-specific data destructor routine may result either in lost
>>>>>> storage
>>>>>> (after at least PTHREAD_DESTRUCTOR_ITERATIONS attempts at
>>>>>> destruction) or
>>>>>> in an infinite loop."
>>>>>>
>>>>>> - In jdk8, we did reset the slot value to NULL before Thread exit.
>>>>>> So, in
>>>>>> this case detachCurrentThread() from a pthread_key destructor
>>>>>> should not
>>>>>> have worked at all.
>>>>>>
>>>>>> Could someone from Oracle maybe shed light on this?
>>>>>
>>>>> Please see the following discussion and bug report:
>>>>>
>>>>> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2014-February/010759.html
>>>>>
>>>>> Note I don't follow this list so please include me directly in any
>>>>> follow-ups if needed.
>>>>>
>>>>> Thanks,
>>>>> David
>>
>>
>


More information about the bsd-port-dev mailing list