IPC blobs can entrain a ContentParent

RESOLVED FIXED in Firefox 30, Firefox OS v1.3

Status

()

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: khuey, Assigned: khuey)

Tracking

unspecified
mozilla30
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(blocking-b2g:1.3+, firefox28 wontfix, firefox29 wontfix, firefox30 fixed, b2g-v1.3 fixed, b2g-v1.3T fixed, b2g-v1.4 fixed)

Details

(Whiteboard: [MemShrink])

Attachments

(1 attachment, 2 obsolete attachments)

Created attachment 8371130 [details] [diff] [review]
Patc

Something I've noticed while debugging Bug 963290 is that IPC Blobs can entrain the ContentParent.  I don't think we can rely on the page allowing the blob to be GCd, but I'm not 100% convinced that this is the right fix.
Attachment #8371130 - Flags: review?(bent.mozilla)
Whiteboard: [MemShrink]
Comment on attachment 8371130 [details] [diff] [review]
Patc

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

I think this looks fine! Except:

::: dom/ipc/Blob.cpp
@@ +954,5 @@
>        MOZ_ASSERT(mActor);
>        MOZ_ASSERT(!mSlice);
>        MOZ_ASSERT(!mDone);
>  
> +      NS_ENSURE_TRUE(mActor->Manager(), NS_ERROR_UNEXPECTED);

This is a void function
Attachment #8371130 - Flags: review?(bent.mozilla) → review+
Created attachment 8372666 [details] [diff] [review]
Patch, fixed per review comment.

Fixed up, ready to land. Carrying over bent's r+. (and I would've landed this had the tree been open)
Attachment #8372666 - Flags: review+
Attachment #8372666 - Flags: checkin?
Keywords: checkin-needed
Created attachment 8372668 [details] [diff] [review]
Patch, fixed per review comment.

Now with checkin comment etc.
Attachment #8372666 - Attachment is obsolete: true
Attachment #8372666 - Flags: checkin?
Attachment #8372668 - Flags: review+

Updated

5 years ago
Attachment #8372668 - Flags: checkin?
Attachment #8371130 - Attachment is obsolete: true
Comment on attachment 8372668 [details] [diff] [review]
Patch, fixed per review comment.

Please just use checkin-needed ;)
Attachment #8372668 - Flags: checkin? → checkin+
Will do! (too many flags to keep track of) Thanks for landing this!
https://hg.mozilla.org/mozilla-central/rev/930f356f1a36
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/ee9ac69ab935
status-b2g-v1.3: affected → fixed
status-b2g-v1.4: --- → fixed
status-firefox28: --- → wontfix
status-firefox29: --- → wontfix
status-firefox30: --- → fixed
status-b2g-v1.3T: --- → fixed
You need to log in before you can comment on or make changes to this bug.