Closed Bug 1212986 Opened 9 years ago Closed 9 years ago

Background ChildImpl does not delete its transport

Categories

(Core :: IPC, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
e10s + ---
firefox44 --- fixed

People

(Reporter: mccr8, Assigned: mccr8)

References

Details

(Keywords: memory-leak, Whiteboard: [MemShrink:P2])

Attachments

(1 file)

      No description provided.
Blocks: 1212987
No longer blocks: 1212987
tracking-e10s: --- → ?
Keywords: mlk
Whiteboard: [MemShrink]
Blocks: 1212987
Comment on attachment 8672091 [details] [diff] [review]
BackgroundChildImpl should delete its Transport.

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

Asking Blake to take this since I don't understand whether this should go in BackgroundChildImpl or ChildImpl.
Attachment #8672091 - Flags: review?(wmccloskey) → review?(mrbkap)
(In reply to Bill McCloskey (:billm) from comment #2)
> Asking Blake to take this since I don't understand whether this should go in
> BackgroundChildImpl or ChildImpl.

Yeah, the deletion in the parent happens in ParentImpl, so maybe that would make more sense, if only for symmetry. But I have no idea about why these classes are split as they are.
Whiteboard: [MemShrink] → [MemShrink:P2]
Comment on attachment 8672091 [details] [diff] [review]
BackgroundChildImpl should delete its Transport.

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

I'm going to mark r+ but I think the call should go in the ChildImpl (which matches the parent side where it happens in ParentImpl::MainThreadActorDestroy).

::: ipc/glue/BackgroundChildImpl.cpp
@@ +99,5 @@
>  {
>    // May happen on any thread!
> +
> +  XRE_GetIOMessageLoop()->PostTask(FROM_HERE,
> +                                   new DeleteTask<Transport>(GetTransport()));

I think the idea is that BackgroundChildImpl is a thin veneer only used to create new concrete child protocols and ChildImpl is the real brains of the operation. Therefore, this should probably go in the ChildImpl.
Attachment #8672091 - Flags: review?(mrbkap) → review+
I moved the PostTask to ChildImpl.
Summary: BackgroundChildImpl does not delete its transport → Background ChildImpl does not delete its transport
https://hg.mozilla.org/mozilla-central/rev/fdd5ebadfbed
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: