Skip to content
This repository was archived by the owner on Mar 8, 2020. It is now read-only.

Commit eb28533

Browse files
committed
Gets rid of the rest of leaks from #113
Signed-off-by: ncordon <[email protected]>
1 parent 7738d78 commit eb28533

File tree

1 file changed

+13
-8
lines changed

1 file changed

+13
-8
lines changed

src/main/native/org_bblfsh_client_v2_libuast_Libuast.cc

+13-8
Original file line numberDiff line numberDiff line change
@@ -461,11 +461,10 @@ class Interface : public uast::NodeCreator<Node *> {
461461
}
462462

463463
// toJ returns a JVM object associated with a node.
464+
// It borrows the reference
464465
jobject toJ(Node *node) {
465466
if (!node) return nullptr;
466-
// TODO(#113) investigate, it looks like a potential memory leak
467-
jobject obj = getJNIEnv()->NewGlobalRef(node->obj);
468-
return obj;
467+
return node->obj;
469468
}
470469

471470
// abstract methods from NodeCreator
@@ -509,7 +508,7 @@ class Interface : public uast::NodeCreator<Node *> {
509508
};
510509

511510
// toJ returns a JVM object associated with a node.
512-
// Returns a new reference.
511+
// Returns a borrowed reference.
513512
jobject Node::toJ() { return iface->toJ(this); }
514513

515514
// lookupOrCreate either creates a new object or returns existing one.
@@ -523,7 +522,7 @@ class Context {
523522
uast::Context<Node *> *ctx;
524523

525524
// toJ returns a JVM object associated with a node.
526-
// Returns a new reference.
525+
// Borrows the reference.
527526
jobject toJ(Node *node) {
528527
if (!node) return nullptr;
529528
return iface->toJ(node);
@@ -548,10 +547,10 @@ class Context {
548547
}
549548

550549
// RootNode returns a root UAST node, if set.
551-
// Returns a new reference.
550+
// Returns a borrowed ref
552551
jobject RootNode() {
553552
Node *root = ctx->RootNode();
554-
return toJ(root); // new ref
553+
return toJ(root); // borrowed ref
555554
}
556555

557556
// Iterate returns iterator over an external UAST tree.
@@ -853,8 +852,14 @@ JNIEXPORT jobject JNICALL Java_org_bblfsh_client_v2_NodeExt_load(JNIEnv *env,
853852
jobject self) {
854853
auto ctx = new Context();
855854
jobject node = ctx->LoadFrom(self);
855+
// We need to make a local reference to node since ctx is going to be destroyed
856+
// before returning, and the global references that each Node carries with it.
857+
// If we do not copy node in a local ref, we return a null, whereas copying it in
858+
// a local ref ensures the native part returns the value to Java and after that
859+
// disposes of the local refs
860+
jobject result = getJNIEnv()->NewLocalRef(node);
856861
delete (ctx);
857-
return node;
862+
return result;
858863
}
859864

860865
JNIEXPORT jobject JNICALL Java_org_bblfsh_client_v2_NodeExt_filter(

0 commit comments

Comments
 (0)