1709138 - Consistent crash loading arewefastyet.com in TemporaryIPCBlobChild::AskForBlob()

Status: NEW

(Core :: DOM: File, defect, P3)

Description

[Randell Jesup [:jesup] (needinfo me)]

100% reproducible for me, multiple browser instances. We crash in TemporaryIPCBlobChild::AskForBlob() because 'this' is null. This is because mActor is null, which (from an rr trace) is caused by an ErrorPropagationRunnable nulling out mActor.

From crashstats, this has been happening for > 6 months - though awfy just started triggering it.

I have the rr session sitting open if needed.

I added logs to see why we were creating the ErrorPropegationRunnable; I got this:
Blob write failed: length 2704475, wrote -1, error -5956 (repeated about 20 times)

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

Fails on 4/26 linux nightly, and on a local linux build build on 15253a3168d2 (appears to be a pull 4/26). Doesn't fail on windows 5/3 nightly or windows 4/26 nightly

Comment 2

[Andrew Sutherland [:asuth] (he/him)]

(In reply to Randell Jesup [:jesup] (needinfo me) from comment #0)

I have the rr session sitting open if needed.

Can you pernosco-submit it?

Clearing :baku NI as :baku doesn't directly work on this code anymore although has been very generous in trying to mentor those who would!

Comment 3

[Randell Jesup [:jesup] (needinfo me)]

It wasn't recorded with the disable-cpuid-features option, so I can't submit it. I do have the trace still, though I've done updates/rebuilds in the repo

Comment 4

[Jens Stutte [:jstutte]]

There are several mActor = nullptr and it seems that we should transform some or all MOZ_ASSERT(mActor); into real if (mActor). It might not be trivial to reason about proper error handling, but most possible options are hopefully better than a crash?

Comment 5

[Jens Stutte [:jstutte]]

Hi Randell, I was trying to reproduce under linux, but was not able to do so with the latest nightly. Does this still reproduce for you (and are there any details in the STR beyond loading https://arewefastyet.com/) ? In case would it be possible to get a pernosco-compatible trace ?
I still think that comment 4 is valid, but to be able to reproduce would be great.
Thanks

Comment 6

[Randell Jesup [:jesup] (needinfo me)]

I cannot repro anymore

Comment 7

[Kacper Michajłow [:kasper93]]

I can reproduce the same crash on https://arewefastyet.com/ and other site that loads a lots of blobs.

What's interesting, while the tab crashes, very rarely it generates crash report, just have "restore tab", button. But once in a while it will crash with full report with the same signature as this issue.

As for STR, just loading https://arewefastyet.com/ is enough, if it doesn't crash immediately, you can change platform and it will likely crash.

Comment 8

[Kacper Michajłow [:kasper93]]

Ok, so it crashes when blob temporary file fails to write to the disk. I think it should be more resilient to such scenario and should crash whole process.

Also I find it pretty heavy to write every blob to the temp folder. Shouldn't we have at least an option to keep it in memory instead?