1309051 - Assert if GetSerialNumber() doesn't find anything when we expect it to (or vice versa)

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Andrew McCreight [:mccr8]]

GetSerialNumber() can be called when we're creating an object (aCreate is true) or when we're destroying an object (aCreate is false). We should assert if we're creating an object and there's already a serial number for this object. We should also assert if we're destroying an object and a serial number does not exist for this object. Doing this helped me track down bug 1271182.

Comment 1

[Andrew McCreight [:mccr8]]

I also implemented some stuff for dumping the allocation and destruction stack in the case of an apparently double free, but it wasn't very useful in the end, so I'm not going to bother landing it.

Comment 2

[Andrew McCreight [:mccr8]]

Try run with this and logging enabled for IPC::Message didn't turn up anything:
  https://treeherder.mozilla.org/#/jobs?repo=try&revision=4ad08fc67e079c8543d056a12bf12d368e4e521b

Comment 3

[Andrew McCreight [:mccr8]]

Attachment: Bug 1309051 - Ensure GetSerialNumber is consistent with aCreate.

Review commit: https://reviewboard.mozilla.org/r/84922/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/84922/

Comment 4

[Andrew McCreight [:mccr8]]

Comment on attachment 8799791 [details]
Bug 1309051 - Ensure GetSerialNumber is consistent with aCreate.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/84922/diff/1-2/

Comment 5

[Andrew McCreight [:mccr8]]

I think MOZ_RELEASE_ASSERT() makes sense, in the unlikely event that somebody is not using DEBUG for the refcounting.

Comment 6

[Nathan Froyd [:froydnj]]

Comment on attachment 8799791 [details]
Bug 1309051 - Ensure GetSerialNumber is consistent with aCreate.

https://reviewboard.mozilla.org/r/84922/#review83460

The argument for using `MOZ_RELEASE_ASSERT` is that somebody might be using ctor/dtor logging with a non-debug build?  That would be kinda of weird, but plausible, given that we export `NS_LogCtor` and friends.  Maybe add a comment to that effect in `GetSerialNumber`, explaining why we're using a release assert?

Comment 7

[Andrew McCreight [:mccr8]]

Comment on attachment 8799791 [details]
Bug 1309051 - Ensure GetSerialNumber is consistent with aCreate.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/84922/diff/2-3/

Comment 8

[Andrew McCreight [:mccr8]]

> Maybe add a comment to that effect in `GetSerialNumber`, explaining why we're using a release assert?
done

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7f5c5ab80c50
Ensure GetSerialNumber is consistent with aCreate. r=froydnj

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7f5c5ab80c50