Thread::current() and JNI pthread interaction

[David Holmes]

On 6/02/2014 2: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.

Right so - I'm assuming the sequence is pthread_exit -> TLS destructor 
-> detachCurrentThread ? And the VM also uses a pthread_key for its 
internal TLS implementation. According to the POSIX spec this seems okay 
on the face of it - only keys with destructors get cleared. But I could 
imagine an implementation that simply walks through the internal key 
table nulling out values and calling destructors if they exist. If that 
were done then there is a real risk that the VM's key will be nulled 
before the destructor runs and then detachCurrentThread will hit the 
null value and crash. My reading of the POSIX spec says this shouldn't 
happen, but perhaps this is how it is implemented on Linux (and BSD) ?

In general though I would say that trying to detach from the VM during 
pthread_exit is fraught with peril - we have no idea what tear-down the 
pthreads library may have already done and whether subsequent use of 
pthreads functionality during the detach will succeed or not. So caveat 
emptor on that part.

But I don't understand the 32-bit versus 64-bit issue because AFAICS we 
do exactly the same thing on both. ??

> 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.

Seems reasonable.

David
-----

> 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
>>
>