Closed Bug 767082 Opened 7 years ago Closed 2 years ago

WebSMS: sSmsChild leaks at shutdown

Categories

(Core :: DOM: Device Interfaces, defect, minor)

defect
Not set
minor

Tracking

()

RESOLVED INCOMPLETE

People

(Reporter: dougt, Assigned: vicamo)

Details

Attachments

(1 file)

Summary: sSmsChild leaks at shutdown → WebSms: sSmsChild leaks at shutdown
Summary: WebSms: sSmsChild leaks at shutdown → WebSMS: sSmsChild leaks at shutdown
Assignee: mounir → nobody
Attached patch patchSplinter Review
1. Make SmsChild refcounted
2. Move GetSendMmsMessageRequestFromParams() into anonymous namespace because it's only referenced in current entity.
Assignee: nobody → vyang
Attachment #770851 - Flags: review?(bent.mozilla)
Comment on attachment 770851 [details] [diff] [review]
patch

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

Hi Vicamo, sorry for taking so long to get to this patch.

I don't really understand why we're making SmsChild refcounted though... It's not possible for SmsChild to live beyond IPDL's normal actor lifetime, is it?
(In reply to ben turner [:bent] from comment #2)
> Hi Vicamo, sorry for taking so long to get to this patch.

I know you're busy :)

> I don't really understand why we're making SmsChild refcounted though...
> It's not possible for SmsChild to live beyond IPDL's normal actor lifetime,
> is it?

Then I'm a bit confused about the meaning of this bug.  If IPDL does free child protocols at shutting down, why do we have sSmsChild "leak"?  Or, did Doug Turner mean we have a uncleared, invalid pointer?
Flags: needinfo?(doug.turner)
Well, in general, the only case for adding reference counting to an actor implementation is if the implementation's lifetime (e.g. the C++ object that implements the actor methods) is somehow different from the actor's lifetime. This situation definitely arises from time to time but I don't think it's really relevant here.

I'm not entirely sure what this bug is about but it's possible that we could end up with a dangling pointer here in some weird case. The simplest thing to do here I think is to make SmsChild::ActorDestroy call a static method on SmsIPCService that simply nulls out gSmsChild. Then we should be bulletproof I think.
Flags: needinfo?(doug.turner)
So I assume Ben's suggestion is enough for this bug.
Cleaning up Device Interfaces component, and mass-marking old FxOS bugs as incomplete.

If any of these bugs are still valid, please let me know.
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.