Closed Bug 1309051 Opened 3 years ago Closed 3 years ago

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

Categories

(Core :: XPCOM, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: mccr8, Assigned: mccr8)

References

Details

Attachments

(1 file)

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.
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.
Try run with this and logging enabled for IPC::Message didn't turn up anything:
  https://treeherder.mozilla.org/#/jobs?repo=try&revision=4ad08fc67e079c8543d056a12bf12d368e4e521b
I think MOZ_RELEASE_ASSERT() makes sense, in the unlikely event that somebody is not using DEBUG for the refcounting.
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?
Attachment #8799791 - Flags: review?(nfroyd) → review+
> Maybe add a comment to that effect in `GetSerialNumber`, explaining why we're using a release assert?
done
Pushed by amccreight@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7f5c5ab80c50
Ensure GetSerialNumber is consistent with aCreate. r=froydnj
https://hg.mozilla.org/mozilla-central/rev/7f5c5ab80c50
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
No longer blocks: 1316527
Depends on: 1316527
Depends on: 1321338
You need to log in before you can comment on or make changes to this bug.