trafodion-codereview mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From zellerh <...@git.apache.org>
Subject [GitHub] incubator-trafodion pull request #697: [TRAFODION-2218] Memory leak from JVM...
Date Wed, 14 Sep 2016 15:42:45 GMT
Github user zellerh commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/697#discussion_r78775500
  
    --- Diff: core/sql/langman/LmRoutineJavaObj.cpp ---
    @@ -192,6 +192,7 @@ LmResult LmRoutineJavaObj::invokeRoutineMethod(
               (*emitRowPtr_)(NULL,0,&qs);
             }
         }
    +  jni->DeleteLocalRef(jniResult);
    --- End diff --
    
    The idea of this code was to have two C++ calls that need to be made directly one after
another:
    
    invokeRoutineMethod() // makes JNI call, returns length of output byte strings
    getRoutineInvocationInfo() // retrieves the output byte strings
    
    The local refs are created by the first call and used by the second. They are cleaned
up either by the next call to invokeRoutineMethod() or by the destructor.
    
    You are right that if a caller does other JNI calls between those two calls they might
get stale objects. I can rewrite this and add an extra copy of the objects. Right now this
is not a problem, but we can change this if you prefer. Alternatively I can add comments to
make it clear these calls have to follow each other, without other JNI calls in-between.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message