> On May 27, 2014, 4:47 p.m., Dominic Hamon wrote: > > src/jvm/jvm.cpp, line 23 > > <https://reviews.apache.org/r/21598/diff/1/?file=583937#file583937line23> > > > > please don't introduce static non-POD objects. See > > http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Static_and_Global_Variables > > for details and potential workarounds. > > Timothy St. Clair wrote: > So... According to *this rule, the code is in violation all over the > place. > > I notice it most heavily in the java objects. I can change it, but even > the Jvm object is in violation. > ref1 - "have no dynamic initialization or destruction." > ref2 - "Likewise, global and static variables are destroyed when the > program terminates, regardless of whether the termination is by returning > from main() or by calling exit()" > > What I would prefer to do is: > static memory::shared_ptr<DynamicLibrary> libJvm; > static memory::shared_ptr<Jvm> instance; > > & actually have proper cleanup, b/c the Jvm object leaks. > > > > > Dominic Hamon wrote: > It is - see MESOS-1023. > > For now we can avoid adding new ones in this review by simply: > > static DynamicLibrary* libJvm() > { > static DynamicLibrary* libJvm = new DynamicLibrary(); > return libJvm; > } > > Note that memory::shared_ptr has an equivalent issue as it is non-POD > (has logic in the destructor). > > This is not just a style guide issue: We had a real bug in the past due > to destruction order of non-PODs.
I can do this, but then we'll leak both the Jvm and libJvm on shutdown. This just seems wrong, as the whole reason was to address leaks on shutdown. I'm fully aware of the convention, but: - I'm not a fan if it inhibits proper behavior. - And again we violate it in other places. - Timothy ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21598/#review43981 ----------------------------------------------------------- On May 17, 2014, 2:02 a.m., Timothy St. Clair wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/21598/ > ----------------------------------------------------------- > > (Updated May 17, 2014, 2:02 a.m.) > > > Review request for mesos, Benjamin Hindman and Niklas Nielsen. > > > Bugs: MESOS-1354 > https://issues.apache.org/jira/browse/MESOS-1354 > > > Repository: mesos-git > > > Description > ------- > > Fix for the coverity leak and update the code to leverage > stout/dynamiclibrary. > > > Diffs > ----- > > src/jvm/jvm.cpp 745b48f > > Diff: https://reviews.apache.org/r/21598/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Timothy St. Clair > >
