[
https://issues.apache.org/jira/browse/HDFS-10257?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15226681#comment-15226681
]
Chris Nauroth commented on HDFS-10257:
--------------------------------------
I disagree with the proposed change.
Even when we use {{__thread}} to avoid mutex contention, we still have a
reliance on the pthreads destructor function to detach the thread from the JVM.
This happens in os/posix/thread_local_storage.c. According to the
documentation of
[{{pthread_key_create}}|http://pubs.opengroup.org/onlinepubs/009695399/functions/pthread_key_create.html],
the destructor function is only called if there is a non-null value associated
with the TLS key.
bq. At thread exit, if a key value has a non-NULL destructor pointer, and the
thread has a non-NULL value associated with that key, the value of the key is
set to NULL, and then the function pointed to is called with the previously
associated value as its sole argument.
Therefore, we still need to call {{threadLocalStorageSet}} to make sure a value
is associated with the key and it can be accessed by the thread destructor.
Please let me know if I missed something or perhaps misunderstood some subtlety
of pthreads.
> Quick Thread Local Storage set-up has a small flaw
> --------------------------------------------------
>
> Key: HDFS-10257
> URL: https://issues.apache.org/jira/browse/HDFS-10257
> Project: Hadoop HDFS
> Issue Type: Bug
> Components: libhdfs
> Affects Versions: 2.6.4
> Environment: Linux
> Reporter: Stephen Bovy
> Priority: Minor
> Original Estimate: 72h
> Remaining Estimate: 72h
>
> In jni_helper.c in the getJNIEnv function
> The “THREAD_LOCAL_STORAGE_SET_QUICK(env);” Macro is in the wrong
> location;
> It should precede the “threadLocalStorageSet(env)” as follows ::
> THREAD_LOCAL_STORAGE_SET_QUICK(env);
> if (threadLocalStorageSet(env)) {
> return NULL;
> }
> AND IN “thread_local_storage.h” the macro:
> “THREAD_LOCAL_STORAGE_SET_QUICK”
> should be as follows ::
> #ifdef HAVE_BETTER_TLS
> #define THREAD_LOCAL_STORAGE_GET_QUICK() \
> static __thread JNIEnv *quickTlsEnv = NULL; \
> { \
> if (quickTlsEnv) { \
> return quickTlsEnv; \
> } \
> }
> #define THREAD_LOCAL_STORAGE_SET_QUICK(env) \
> { \
> quickTlsEnv = (env); \
> return env;
> }
> #else
> #define THREAD_LOCAL_STORAGE_GET_QUICK()
> #define THREAD_LOCAL_STORAGE_SET_QUICK(env)
> #endif
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
