Henry Robinson has posted comments on this change.

Change subject: Fix a potential crash in Frontend & Catalog JNI startup
......................................................................


Patch Set 1:

I don't fully understand the JNI model, but I'm wondering if this is exactly 
the right fix. Looking at 
https://github.com/apache/incubator-impala/blob/master/be/src/util/backend-gflag-util.cc#L73,
 it seems to me that cfg_bytes is allocated on a local frame instantiated in 
GetThriftBackendGflags(). That frame is then popped at the end of the method, 
so does cfg_bytes then go out of scope at that point (i.e. earlier than this 
patch assumes)? 

Maybe try forcing a GC after GetThriftBackendGflags() to see if the reference 
is collected? I admit to not really understanding why the local frame is 
needed, so I'm probably missing something.

-- 
To view, visit http://gerrit.cloudera.org:8080/6264
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I35398a8efdb6fdbf7932a32489b2ad8d99b6d76f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <[email protected]>
Gerrit-Reviewer: Henry Robinson <[email protected]>
Gerrit-HasComments: No

Reply via email to