On Mon, Aug 04, 2014 at 04:09:26PM -0700, Kyle Huey wrote:
> On Mon, Aug 4, 2014 at 4:06 PM, Mike Hommey <m...@glandium.org> wrote:
> > On Mon, Aug 04, 2014 at 03:42:07PM -0700, Kyle Huey wrote:
> >> On Mon, Aug 4, 2014 at 3:09 PM, Mike Hommey <m...@glandium.org> wrote:
> >> > On Mon, Aug 04, 2014 at 11:33:55AM -0700, Kyle Huey wrote:
> >> >> On Sun, Aug 3, 2014 at 11:01 PM, Nicholas Nethercote
> >> >> <n.netherc...@gmail.com> wrote:
> >> >> > On Sun, Aug 3, 2014 at 10:18 PM, Kyle Huey <m...@kylehuey.com> wrote:
> >> >> >> Static*Ptrs are there to avoid
> >> >> >> static constructors and destructors so they can't clear themselves at
> >> >> >> shutdown.  That means that they behave quite differently than
> >> >> >> "regular" smart pointers.  Am I the only one who is bothered by this?
> >> >> >
> >> >> > To use them correctly, is it the case that you need to do one of the 
> >> >> > following?
> >> >> >
> >> >> > - null them at some point, or
> >> >> >
> >> >> > - call ClearOnShutdown() on them at some point.
> >> >> >
> >> >> > The comments in StaticPtr.h don't make this clear. And
> >> >> > ClearOnShutdown.h is a separate file, and ClearOnShutdown() isn't
> >> >> > mentioned in StaticPtr.h.
> >> >> >
> >> >> > So I think they're useful -- being able to avoid static constructors
> >> >> > is important. But the documentation on how to use them could be much
> >> >> > clearer.
> >> >> >
> >> >> > Nick
> >> >>
> >> >> Yes, that's correct.  You have to clear them somehow before the
> >> >> process exits or we leak.
> >> >
> >> > Which, besides accounting, is not really a problem, since the process is
> >> > exiting anyways. It can be a problem if desctructor code is actively
> >> > doing import stuff like persisting state.
> >> >
> >> > Mike
> >>
> >> That accounting is useful though.  We need to fix the leaks that don't
> >> matter in order to catch the ones that do, so that the latter don't
> >> get lost in the noise of the former.
> >
> > That accounting is useful on debug builds. Arguably, debug builds don't
> > have to care about avoiding static initializers. So we could add a
> > destructor to Static*Ptrs on debug builds that:
> > - sends a error that ClearOnShutdown was not called on them
> > or
> > - clears them to clear the leak
> > or
> > - both (replacing the error with a warning)
> >
> > Mike
> 
> Yeah, we could do that.  But turning on leak checking does the same
> thing as (a) so (b) is the only useful code change to make.

Well, it only does (a) indirectly. That is, it's not obvious. An
explicit warning about what's missing would be more useful.

Mike
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Reply via email to