846368 - Add number of bytes requested to OOM last-gasp

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Adam Roach [:abr]]

Currently, when memory allocation fails, the only diagnostic printed to the
log messages is a simple "out of memory" string. It would be useful in
distinguishing from a true out-of-memory situation and a request for
an unrealistically large block of memory if the message included the
size of the requested allocation.

Comment 1

[Adam Roach [:abr]]

Attachment: Add number of bytes requested to OOM last-gasp

This patch uses a preallocated buffer and primitive operations to maximize the
chance of success under potentially unstable conditions, as might exist in
true out-of-memory situations.

Comment 2

[Adam Roach [:abr]]

Comment on attachment 719534 [details] [diff] [review]
Add number of bytes requested to OOM last-gasp

Benjamin: I'm just taking a guess that you're the right person to look at this patch. If it needs additional or different eyes, please let me know.

Comment 3

[Benjamin Smedberg]

Which message are we talking about? The one printed in crash-stats, or something on the console?

Comment 4

[Adam Roach [:abr]]

On the console. This might be clearer if I give an example. Look at this tbpl log:

https://tbpl.mozilla.org/php/getParsedLog.php?id=20159969&tree=Mozilla-Inbound#error1

In cases like this, I'd like the line "15:29:23     INFO -  out of memory" to provide more useful diagnostics.

BTW, reading through the log for Bug 811483, I note that this really needs to be thread-safe. I was hoping to avoid pushing another 60 bytes or so onto the stack in what could be a delicate operating state, but any other solution would be even more disruptive. I'm about to update the patch to put the message buffer on the stack to handle potential threading issues.

Comment 5

[Adam Roach [:abr]]

Attachment: Add number of bytes requested to OOM last-gasp

Comment 6

[Benjamin Smedberg]

Comment on attachment 719550 [details] [diff] [review]
Add number of bytes requested to OOM last-gasp

ok, that makes sense. I'm pretty sure that this will not affect the abort message in crash stats (which is desired, because signatures might change).

Have you any interest in taking bug 811483?

Comment 7

[Benjamin Smedberg]

Actually... this is printing in decimal. Perhaps it would make more sense to print in hex?

Comment 8

[Adam Roach [:abr]]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #6)
> Have you any interest in taking bug 811483?

Under other circumstances, I'd say yes -- but right now, we're in a pretty frantic push to get the WebRTC stuff stable and finished.

Comment 9

[Adam Roach [:abr]]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #7)
> Actually... this is printing in decimal. Perhaps it would make more sense to
> print in hex?

Sure, that would be easy enough. I'll respin the patch.

Comment 10

[Adam Roach [:abr]]

Attachment: Add number of bytes requested to OOM last-gasp

Comment 11

[Adam Roach [:abr]]

Attachment: Add number of bytes requested to OOM last-gasp

Comment 12

[Adam Roach [:abr]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0644287fbbdc

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0644287fbbdc