Thread::current() and JNI pthread interaction

Thu Feb 6 07:36:24 PST 2014

Gerard, David Holmes, thanks for looking at this.

Gerard, I will make sure to fix it in the BSD files as well, thanks.

David Holmes, looking at the POSIX spec I read it the same way you do, 
but both the Linux and the Mac implementation seem to be just nulling 
out everything in order.

I looked a bit more at how restoring the thread pointer with a 
destructor affected execution.
There are three different scenarios:

1) detachCurrentThread is called from the main logic of native code:
detachCurrentThread calls Thread::~Thread, which sets the TLS thread 
pointer to NULL.
Pthread never calls the destructor (it only calls for non-NULL values).
This is also what is done when a Java thread is spawned by the VM itself.
This works out the same way as if we didn't have a thread pointer 
destructor.

2) detachCurrentThread is called from a pthread TLS destructor:
The thread pointer destructor runs first and restores the thread pointer 
value.
detachCurrentThread is called from the native code destructor.
Thread::~Thread is called from detachCurrentThread and sets the TLS 
thread pointer to NULL.
The thread pointer destructor is not called again, since its value is 
NULL now.
This is neat since no new state needs to be tracked.

3) detachCurrentThread is never called:
The thread pointer destructor will loop PTHREAD_DESTRUCTOR_ITERATIONS 
times (or possibly forever), unless some extra state is tracked.
The Thread object will be leaked, as before. In theory (looking at the 
POSIX spec) we could leak additional TLS space, but at least the Linux 
implementation still cleans up the TLS.

My opinion is that it isn't worth adding more code to avoid leaking TLS 
space or looping the thread pointer destructor,
since the only case where that can happen is if detachCurrentThread is 
never called (which is already bad).

What do you think?

Regards,
Andreas

On 2014-02-06 00:04, Gerard Ziemski wrote:
> hi Andreas,
>
> I just tested the issue and it's present on Mac as well - I updated 
> your bug and also linked related NetBeans on Ubuntu64 bit crash.
>
> I have tested your fix on Mac and it seems fine to me (not an official 
> reviewer), though using a pthread_key destructor to reset that same 
> pthread_key value back seems a little bit counter-intuitive, so that 
> pthread_key destructor could probably benefit from a nice comment. I'm 
> unclear on one thing: in the end, do we end up leaking the pthread_key 
> and if so is that a big deal or not?
>
> I believe the final fix will need to applied to Mac platform as well 
> as Linux.
>
>
> cheers
>
> On 2/5/2014 10:38 AM, Andreas Eriksson wrote:
>> Thanks for looking at this.
>>
>> I filed a bug at https://bugs.openjdk.java.net/browse/JDK-8033696.
>> A correction to my original mail is that it is not Thread::~Thread 
>> that resets the thread pointer in the TLS, it is pthreads itself.
>>
>> 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.
>>
>> Regards,
>> Andreas
>>
>> On 2014-02-05 17:15, David Simms wrote:
>>>
>>> This looks like a bug unfortunately, the JNI documentation doesn't 
>>> appear to warn against this kind of use.
>>>
>>> The code in question:
>>> 	static void make_key()
>>> 	{
>>> 		pthread_key_create(&key, detachThread);
>>> 	}
>>>
>>> 	
>>> 	/**
>>> 	 *  pthread key destructor (runs after the JVM current thread key is already destroyed)
>>> 	 */
>>> 	static void detachThread(void *p)
>>> 	{
>>> 		if (p != 0)
>>> 		{
>>> 			JavaVM *jvm = 0;
>>> 			JNIEnv *env = (JNIEnv *) p;
>>> 			env->GetJavaVM(&jvm);
>>> 			*jint result = jvm->DetachCurrentThread();*
>>>
>>> The JVM current thread is already toast as you suggest.
>>>
>>> I believe Andreas that you might already have a simple pain free 
>>> work-around from the JVM. Given that the problem only appears on 
>>> certain platforms, there may be other case where people migrate to 
>>> 64 bit and find this nasty crash. Be worth filing a bug...
>>>
>>> Cheers
>>> /David Simms
>>>
>>>
>>> On 02/05/2014 12:47 PM, Andreas Eriksson wrote:
>>>> Hi,
>>>>
>>>> I'm investigating a bug where JNI code (attached, compilation 
>>>> instructions below) is using pthread_key_create with a destructor 
>>>> to detach the thread from the JVM when the thread is exiting.
>>>> This solution works well when running on Solaris or a 32 bit JVM on 
>>>> Linux, but when run on 64 bit JVM on Linux the threads hang when 
>>>> detaching.
>>>>
>>>> It turns out that for 64 bit Linux the JVM is also using the 
>>>> pthread_key_create, to store the Thread::current() value in a 
>>>> thread local storage.
>>>> Ssince the thread local storages are reset in Thread::~Thread 
>>>> (ThreadLocalStorage::set_thread(NULL)), before the JNI destructor 
>>>> runs, we run detachCurrentThread on a thread that has NULL as 
>>>> current thread.
>>>> With a product build this breaks locks/monitors, and the threads 
>>>> hang. With a debug build an assert in Thread::current() is hit 
>>>> instead.
>>>>
>>>> Everything works if detachCurrentThread is called from the main 
>>>> logic instead.
>>>>
>>>> Is this considered a bug, or maybe this behavior is expected?
>>>>
>>>> Regards,
>>>> Andreas
>>>>
>>>> Compile native:
>>>> # 64bit
>>>> JAVA_HOME=/java/linux-x64/jdk1.7.0_45
>>>> gcc -shared -fpic  -o libNative.so  -I$JAVA_HOME/include 
>>>> -I$JAVA_HOME/include/linux -lstdc++ Callback_Native.cpp
>>>>
>>>> # 32bit
>>>> JAVA_HOME=/java/linux-i586/jdk1.7.0_45
>>>> gcc -v -m32 -shared -fpic  -o libNative.so -I$JAVA_HOME/include 
>>>> -I$JAVA_HOME/include/linux -lstdc++ Callback_Native.cpp
>>>>
>>>> Compile java (from callback/src/main/java):
>>>> JAVA_HOME=/java/linux-x64/jdk-1.7.0_45
>>>> $JAVA_HOME/bin/javac com/test/callback/CallbackTest.java
>>>> $JAVA_HOME/bin/javac com/test/callback/App.java
>>>>
>>>> To run: (from callback/src/main/java)
>>>> NATIVE=../../../native
>>>> $JAVA_HOME/bin/java -Djava.library.path=$NATIVE com.test.callback.App
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20140206/b594772d/attachment-0001.html 