ClearOnShutdown isn't going to work if you call it after XPCOM shutdown. I guess if it's called early enough after shutdown, the event loops will still be running, and we could post an event to the event loop to clear the pointer soon. But probably better just to assert.
Created attachment 586973 [details] [diff] [review]
Comment on attachment 586973 [details] [diff] [review]
Actually, this is not going to work. It could be that ClearOnShutdown() observes XPCOM shutdown before its caller does. In that case, we'll assert, but the caller never had an opportunity to notice that XPCOM has shut down!
We could expose a "shutting down" bool on xpcom, and set it to true before we fire any shutdown notifications. But then I bet callers are just going to have to guard their ClearOnShutdown() calls with that bool.
Maybe it's better to figure out how to get trace-malloc not to care about these "leaked" objects. That's the whole reason ClearOnShutdown() exists, anyway.
dbaron, you seem to own trace-malloc. Would you be OK with adding an API to trace-malloc which says "treat this object as free'd"?
Created attachment 587084 [details] [diff] [review]
Created attachment 587085 [details] [diff] [review]
Comment on attachment 587085 [details] [diff] [review]
The problem that this fixes is: While XPCOM shutdown notifications are being sent to observers, things can still happen. In particular, it's not outrageous for someone to call ClearOnShutdown() at this point. So we really need to wait until all the event queues are quiet before null'ing out the ClearOnShutdown()'ed pointers.
Accepting suggestions for a better name than "KillClearOnShutdown". I just couldn't bring myself to do "ShutDownClearOnShutdown".
(In reply to Justin Lebar [:jlebar] from comment #3)
> dbaron, you seem to own trace-malloc. Would you be OK with adding an API to
> trace-malloc which says "treat this object as free'd"?
What for? And how does it relate to this bug?
Well, that would be a different way of handling this bug. At least, I thought it would be.
ClearOnShutdown (at least at the moment) only clears pointers for the sake of convincing trace-malloc that the pointer was freed. Nobody relies on the clearing of pointers for correctness.
So my thought was that instead of clearing a pointer, we could tell trace-malloc that the pointer has been freed, and that would be sufficient.
But I realize now that there are at least few problems with this:
* If you have a clear-on-shutdown'ed nsTArray, you need to delete that object so it deletes its buffer. If we told trace-malloc that the TArray was cleared, it would still think we were leaking the buffer.
* It's a sign of a problem if an object doesn't get deleted after its clear-on-shutdown'ed pointer is deleted. That means someone else is holding a reference to the object, probably incorrectly. Again, the current implementation would detect this, while the one suggested in this comment wouldn't.
Is there a testcase for this bug which QA can use to verify the fix?
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #11)
> Is there a testcase for this bug which QA can use to verify the fix?
No; this was precautionary.