hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Andy Isaacson (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-799) libhdfs must call DetachCurrentThread when a thread is destroyed
Date Wed, 11 Jul 2012 22:05:34 GMT

    [ https://issues.apache.org/jira/browse/HDFS-799?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13412052#comment-13412052
] 

Andy Isaacson commented on HDFS-799:
------------------------------------

{code}
-/**
- * getJNIEnv: A helper function to get the JNIEnv* for the given thread.
- * If no JVM exists, then one will be created. JVM command line arguments
- * are obtained from the LIBHDFS_OPTS environment variable.
- *
- * @param: None.
- * @return The JNIEnv* corresponding to the thread.
- */
-JNIEnv* getJNIEnv(void)
+JNIEnv* getGlobalJNIEnv(void)
 {
{code}
we need a new function comment here. ... ah I see below that it's now a helper; make it static?
 Then you don't need to add a function comment.
{code}

@@ -397,12 +420,12 @@ JNIEnv* getJNIEnv(void)
 
     // Only the first thread should create the JVM. The other trheads should
     // just use the JVM created by the first thread.
-    LOCK_JVM_MUTEX();
+    pthread_mutex_lock(&jvmMutex);
{code}
Even though this is the only callsite, the macro performs a nice documentary function.  I
don't think there's much benefit to unmacroizing this code.
{code}
+    tls = calloc(1, sizeof(struct hdfsTls));
+    if (!tls) {
+        fprintf(stderr, "getJNIEnv: OOM\n");
{code}
OOM => "calloc(%d) failed" or similar.

{code}
+static void _init()
+{
+    pthread_key_create(&gTlsKey, tlsDestructor);
+}
{code}
{{pthread_key_create}} can fail, we should do something useful in that case.  (I don't know
offhand if we are allowed to {{fprintf}} in an {{__attribute__((constructor))}} function...)

Other than that, LGTM.  The fundamental idea is sound and does resolve the problem.

Does this add any additional compiler/linker dependencies?  I know I've had cases in the past
of constructors and destructors not getting called due to link flag badness and/or ld bugs.
 I think the existing code will work (with the memory leak you pointed out) on any JNI + pthreads
implementation; the __attribute__ stuff is a GCC extension.  Which, admittedly, is implemented
by most compilers nowadays, but it's still a potential issue.  (I pity the fool who has to
get this working with {{xlc}} on AIX.)
                
> libhdfs must call DetachCurrentThread when a thread is destroyed
> ----------------------------------------------------------------
>
>                 Key: HDFS-799
>                 URL: https://issues.apache.org/jira/browse/HDFS-799
>             Project: Hadoop HDFS
>          Issue Type: Improvement
>    Affects Versions: 2.0.0-alpha
>            Reporter: Christian Kunz
>            Assignee: Colin Patrick McCabe
>         Attachments: HDFS-799.001.patch, HDFS-799.003.patch
>
>
> Threads that call AttachCurrentThread in libhdfs and disappear without calling DetachCurrentThread
cause a memory leak.
> Libhdfs should detach the current thread when this thread exits.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Mime
View raw message