[B2G][RIL] MobileConnection shouldn't be shutdown if JS still keeps a ref to it.

RESOLVED DUPLICATE of bug 1048581

Status

Firefox OS
RIL
RESOLVED DUPLICATE of bug 1048581
4 years ago
4 years ago

People

(Reporter: edgar, Assigned: edgar)

Tracking

unspecified
2.1 S2 (15aug)
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [p=2])

(Assignee)

Description

4 years ago
+++ This bug was initially created as a follow-up of Bug #843452 comment #153 +++

MobileConnection::Shutdown is currently called
by MobileConnectionArray::DropConnections().
And that is usually called when unlink runs. But what if JS keeps a ref
to a MobileConnection object. It shouldn't be shutdown.
(Assignee)

Updated

4 years ago
Depends on: 946437
(Assignee)

Comment 1

4 years ago
1). The unlink of MobileConnectionArray should just release the ref to MobileConnection. (Don't need
    call MobileConnection::Shutdown)
2). And MobileConnection calls Shutdown in it's dtor.

Comment 2

4 years ago
Or MobileConnection calls Shutdown in unlink.
One thing to check is what keeps MobileConnection alive? Is it registered to some service so that it is
never delete before shutdown? That would mean it is never Shutdown().
If there is such service, it probably shouldn't keep a strong reference to MobileConnection, at least
if MobileConnection doesn't have event listeners for such event which might still happen in the future.
(Scripts shouldn't be able to detect when CC or GC runs).

Since MobileConnection is an EventTarget, Shutdown() should be called when DisconnectFromOwner() is called. That is called when the relevant Window is closed.
(The case is somewhat similar to WebSocket::DisconnectFromOwner())
(Assignee)

Comment 3

4 years ago
(In reply to Olli Pettay [:smaug] from comment #2)
> Or MobileConnection calls Shutdown in unlink.
> One thing to check is what keeps MobileConnection alive? Is it registered to
> some service so that it is
> never delete before shutdown? That would mean it is never Shutdown().

No, MobileConnection isn't registered to some service.
We register MobileConnectionListener to service and MobileConnectionLister doesn't keep a strong reference to MobileConnection.

> If there is such service, it probably shouldn't keep a strong reference to
> MobileConnection, at least
> if MobileConnection doesn't have event listeners for such event which might
> still happen in the future.
Yes, agree. Calling Shutdown in MobileConnection's dtor isn't a good way.

> (Scripts shouldn't be able to detect when CC or GC runs).
> 
> Since MobileConnection is an EventTarget, Shutdown() should be called when
> DisconnectFromOwner() is called. That is called when the relevant Window is
> closed.
> (The case is somewhat similar to WebSocket::DisconnectFromOwner())

Thank for these explanation, it is really helpful.
(Assignee)

Updated

4 years ago
Whiteboard: [p=2]
Target Milestone: --- → 2.1 S2 (15aug)
(Assignee)

Updated

4 years ago
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1048581
You need to log in before you can comment on or make changes to this bug.