> On May 27, 2014, 9:47 a.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.
>
>
>
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.
- Dominic
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21598/#review43981
-----------------------------------------------------------
On May 16, 2014, 7:02 p.m., Timothy St. Clair wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21598/
> -----------------------------------------------------------
>
> (Updated May 16, 2014, 7:02 p.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
>
>
