----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53386/#review154686 -----------------------------------------------------------
src/jvm/jvm.cpp (line 95) <https://reviews.apache.org/r/53386/#comment224360> Please either `CHECK(!opts.empty())` or make this conditional if (!opts.empty()) { vmArgs.options = &opts[0]; } I wasn't able to find out if `vmArgs` needs to be set to _something_ if `nOptions` is zero (I looked at https://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/invocation.html). The code you replaced sets it to the address of a zero-sized array (which is not a `nullptr`), but there is no direct equivalent when using a `vector`. It seems `CreateJavaVM` wouldn't be able to do anything with the passed `options` if `nOptions` was zero, so not setting it to anything might be safe. - Benjamin Bannier On Nov. 3, 2016, 12:56 a.m., Neil Conway wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53386/ > ----------------------------------------------------------- > > (Updated Nov. 3, 2016, 12:56 a.m.) > > > Review request for mesos and Joseph Wu. > > > Repository: mesos > > > Description > ------- > > Fixed memory leak in JVM code. > > > Diffs > ----- > > src/jvm/jvm.cpp 62f7857180162c510269b5d10f5add94ab354fa2 > > Diff: https://reviews.apache.org/r/53386/diff/ > > > Testing > ------- > > `make check` > > Verified that observed `clang-tidy` leak warning goes away with this change. > > > Thanks, > > Neil Conway > >
