1683288 - Improve the mechanism used to set the OOM allocation size

Status: RESOLVED FIXED

(Toolkit :: Crash Reporting, enhancement)

Description

[Gabriele Svelto [:gsvelto] (PTO)]

See bug 1682975 comment 7 for why we would want to do this.

Right now we pass to the memory allocator a pointer to a function that gets called when we hit an OOM. The function takes the size of the allocation that couldn't be satisfied and sets a global variable in the crash reporting code. If the function pointer isn't set we don't set the annotation.

I was wondering if flipping things around would improve the situation: having a global variable in the memory allocator code that is set when we hit an OOM scenario with the size that couldn't be satisfied. Then we'd expose a function to read it (or expose the variable itself) and have the exception handler read it at crash time. Something like get_oom_abort_size(). This mechanism would be slightly simpler than set_oom_abort_handler() and since we don't have any other user of that function I think it might just be OK. Mike WDYT about this?

Comment 1

[:gerard-majax]

Attachment: Bug 1683288 - Directly query OOMAllocationSize value and enable tests on linux/macOS r?gsvelto

Comment 2

[:gerard-majax]

Attachment: Bug 1683288 - Directly query OOMAllocationSize value and enable tests on linux/macOS r?gsvelto

Comment 3

[:gerard-majax]

Attachment: Bug 1683288 - Directly query OOMAllocationSize value and enable tests on linux/macOS r?gsvelto

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/24cac1a6750d
Directly query OOMAllocationSize value and enable tests on linux/macOS r=gsvelto

Comment 5

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/24cac1a6750d

Comment 6

[Gabriele Svelto [:gsvelto] (PTO)]

Looking at the nightly channel it seems like that the vast majority of OOM crashes now have the annotation set. The few which don't are those captured by WER and I filed bug 1711418 to fix them. There are still a very small amount of crashes that are still missing the annotation but they're so few that I think we can consider this fixed for good.