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

Status: RESOLVED DUPLICATE of bug 1048581

(Firefox OS Graveyard :: RIL, defect)

Description

[Edgar Chen [:edgar]]

+++ 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.

Comment 1

[Edgar Chen [:edgar]]

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

[Olli Pettay [:smaug][[e-mail]]]

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())

Comment 3

[Edgar Chen [:edgar]]

(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.