Cleaning up SIM hot swap

[Eric Caruso]

I think you're right that mbim-proxy does handle this, since it
disconnects all of the relevant signal handlers when client_unref is
called (via client_set_device). I put up the version of the patch that
I have right now that leaves the cleanup to mbim-proxy; however, I'm
going to want to clean it up more explicitly since we're not using
mbim-proxy in Chrome OS at the moment.

On Mon, Jul 10, 2017 at 11:52 AM, Aleksander Morgado
<aleksander at aleksander.es> wrote:
> On Mon, Jul 10, 2017 at 8:37 PM, Eric Caruso <ejcaruso at chromium.org> wrote:
>>>> > While trying to implement setup_sim_hot_swap for MBIM modems, I
>>>> > noticed that I am setting up and enabling unsolicited events for
>>>> > SUBSCRIBER_INFO type notifications, but not cleaning them up. This
>>>> > might not end up affecting the user since the destruction of the
>>>> > modem
>>>> > object will disconnect the signal handler, but it means I might be
>>>> > leaving the modem in a weird state when it's disabled or unplugged
>>>>
>>>> consider that the modems might not keep this configuration after
>>>> reboot, unless you've explicitly saved it with a proper command. At
>>>> least, this is the behavior of Telit modems, whose plugin is the only
>>>> one that currently implements sim hot swap in modemmanager.
>>>
>>>
>>> Yeah, that's true, and it definitely is the case also for MBIM notifications.
>>>
>>> Plus, even if it's a good idea to request to disable the
>>> SUBSCRIBER_INFO notifications, it isn't critical if it fails, and
>>> therefore you don't really need to chain up 2 async methods: just
>>> issue the async MBIM calls with a NULL callback and go on
>>> synchronously (i.e. you run the MBIM command but don't wait for the
>>> result of the operation). Another option would be to pass a valid
>>> callback BUT also keep on synchronously, the callback would just e.g.
>>> log warning if the operation fails.
>>>
>>
>> It's still not clear where I can safely queue up these asynchronous
>> operations without them possibly being scheduled to run after the
>> destruction of the object and the MBIM port. It seems like the dispose
>> method is the wrong place to do that, as I imagine that all but
>> guarantees that we'll have cleaned up those objects by the time the
>> asynchronous operations run.
>
> I'm tempted to say that finalize() isn't the worst place for this,
> just before we run mm_port_mbim_close(). The only logical moment where
> we want to do this cleanup is when stopping MM while the modem is
> still connected AND there is some other application using the MBIM
> port (so really not that often). In this case we do want to cleanup
> the notifications enabled in our end so that the mbim-proxy and the
> modem handle the correct set. If there is no other app using the MBIM
> port, it is expected that the notifications are reset to defaults when
> we issue a MBIM close (something that happens only when no other app
> is using the MBIM port). Actually, if the mbim-proxy detects that the
> MM socket connection is closed, doesn't the mbim-proxy already disable
> the notifications that were explicitly enabled in the MM end?
>
> --
> Aleksander
> https://aleksander.es