Closed Bug 1310518 Opened 3 years ago Closed 3 years ago

Crash in __CFTypeCollectionRetain

Categories

(Core :: Widget: Cocoa, defect, critical)

52 Branch
x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox49 --- unaffected
firefox50 --- unaffected
firefox51 --- unaffected
firefox52 --- fixed

People

(Reporter: kats, Assigned: Gijs)

References

Details

(Keywords: crash, regression)

Crash Data

Attachments

(2 files)

This bug was filed from the Socorro interface and is 
report bp-e7bb817d-8812-4f7f-8cf1-2f4442161016.
=============================================================

I got this crash after saving a file download. I can reproduce this (2/2 so far anyway) and I'll try to make a reduced test case.
Attached file Test case
Load the file, click the button. It will prompt you to save a file (this is generated in JS). Then the browser crashes. 100% reproducible for me.
I can only repro in Nightly, other channels seem ok.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #1)
> Created attachment 8801598 [details]
> Test case
> 
> Load the file, click the button. It will prompt you to save a file (this is
> generated in JS). Then the browser crashes. 100% reproducible for me.

To clarify the STR, you have to actually save the file before the browser will crash.
Uh... having looked at the changes I made a second time, I don't know why they would cause this. Though actually, I suppose I would have made the code run (like at all) where it would have bailed out earlier, with some of the basic fixes that enabled this for non-release versions. So hopefully this is good news in that we're catching a bug that might have happened on release otherwise?

I'm not sure what the error means. A bit of googling seems to suggest that maybe this type of collection doesn't like it if the value is null, so maybe we should nullcheck file/source/referrer URLs? Apparently either that, or we should create the dictionary differently (not sure if that's possible here because we get the dictionary from someone else and then get a mutable copy/reference somehow)  or we should pass an explicit NSNull instead of an actuall nullptr. Not sure what would be best. Josh has left, so Markus, can you help me out?
Flags: needinfo?(mstange)
Flags: needinfo?(gijskruitbosch+bugs)
Flags: needinfo?(gijskruitbosch+bugs)
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Comment on attachment 8805592 [details]
Bug 1310518 - fix crashes when downloading files without referrer/source URL,

https://reviewboard.mozilla.org/r/89346/#review88526
Attachment #8805592 - Flags: review?(mstange) → review+
Flags: needinfo?(mstange)
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/2caaf35f549b
fix crashes when downloading files without referrer/source URL, r=mstange
https://hg.mozilla.org/mozilla-central/rev/2caaf35f549b
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.