1349419 - Crash in OOM | large | mozalloc_abort | mozalloc_handle_oom | moz_xmalloc | mozilla::MakeUnique<T>

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Ryan VanderMeulen [:RyanVM]]

This bug was filed from the Socorro interface and is 
report bp-944cefe5-1caa-435c-b286-0089a2170319.
=============================================================

Comment 1

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

We probably want a fallible memory alloc here and throw JS exception while OOM.
https://hg.mozilla.org/mozilla-central/annotate/e1576dd8bd9d/netwerk/base/ArrayBufferInputStream.cpp#l39

Comment 2

[Josh Matthews [:jdm]]

Yes, and we should delay setting mBufferLength until the buffer has been successfully allocated or we could end up trying to read from a null buffer.

Comment 3

[Daniel Stenberg [:bagder]]

Attachment: 0001-Bug-1349419-do-fallible-memory-alloc-and-return-fail.patch

Something like this?

Comment 4

[Josh Matthews [:jdm]]

Comment on attachment 8850520 [details] [diff] [review]
0001-Bug-1349419-do-fallible-memory-alloc-and-return-fail.patch

Review of attachment 8850520 [details] [diff] [review]:
-----------------------------------------------------------------

Yep!

Comment 5

[Daniel Stenberg [:bagder]]

Attachment: v2-0001-Bug-1349419-do-fallible-memory-alloc-and-return-f.patch

Updated the commit message

Comment 6

[Daniel Stenberg [:bagder]]

Try-run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=110b5cc29427644c11a2c6dabdb7c076c3db1a71&selectedJob=86280314

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9a714b38cd55
do fallible memory alloc and return failure, r=mcmanus

Comment 8

[Carsten Book [:Tomcat]]

backed out in https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=1e98eaf0f70f7ce12244ac5bf9293048b507bb23&filter-searchStr=Linux+opt+Mochitests+executed+by+TaskCluster+test-linux32%2Fopt-mochitest-browser-chrome-4+tc-M(bc4) it seems that one of this 3 checkins cause times like https://treeherder.mozilla.org/logviewer.html#?job_id=86621316&repo=mozilla-inbound and seems the tests fail as example on connecting -> https://public-artifacts.taskcluster.net/B3lwHCOzQTe1j9McYz1LLg/0/public/test_info//mozilla-test-fail-screenshot_DRpsBk.png

Comment 9

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4ef4320830cd
Backed out changeset 9a714b38cd55

Comment 10

[Daniel Stenberg [:bagder]]

Based on the output, I think we can say that this patch was probably *not* the reason for the breakage. The other two changes are related to that format parsing so they are the ones I suspect. Or perhaps the test itself. I'm digging into this.

Comment 11

[Daniel Stenberg [:bagder]]

The previous backout was due to problems with bug 1344467 and not this. Asking for a renewed attempt.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3d6fd421ac74
Do fallible memory alloc and return failure to avoid OOM crash. r=mcmanus

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/3d6fd421ac74

Comment 14

[Ryan VanderMeulen [:RyanVM]]

AFAICT, the affected code was introduced in bug 1298773. Please request Aurora approval on this when you get a chance.

Comment 15

[Daniel Stenberg [:bagder]]

Comment on attachment 8850896 [details] [diff] [review]
v2-0001-Bug-1349419-do-fallible-memory-alloc-and-return-f.patch

Approval Request Comment
[Feature/Bug causing the regression]: 1349419
[User impact if declined]: bug remains
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: yes (based on crash-stats)
[Needs manual test from QE? If yes, steps to reproduce]: not sure how to repro!
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: tiny change
[String changes made/needed]: none

Comment 16

[Gerry Chang [:gchang]]

Comment on attachment 8850896 [details] [diff] [review]
v2-0001-Bug-1349419-do-fallible-memory-alloc-and-return-f.patch

Fix a crash. Aurora54+.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8da95adec5a7

Comment 18

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8850896 [details] [diff] [review]
v2-0001-Bug-1349419-do-fallible-memory-alloc-and-return-f.patch

I don't see any instances of this crash on ESR52 in the past month or so. Let's not uplift this.