Age | Commit message (Collapse) | Author |
|
We want to ensure that all errors reported via DBus operations are
normalized to MM-specific errors.
We don't want to return QMI or MBIM specific errors, as those are
protocol specific and we don't want DBus clients to need to rely on
knowing which is the protocol in use by the device.
|
|
ModemManager handles suspend and resume signals sent from powerd
Fixes https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/issues/547
|
|
There is no input cancellable in the method, so the GTask will never
get cancelled from the outside.
|
|
On resume, fetch the current network time as
the network time may be changed when suspended.
|
|
==226546== Invalid read of size 4
==226546== at 0x5068CB7: g_error_matches (in /usr/lib/libglib-2.0.so.0.6600.7)
==226546== by 0x1BAC86: load_network_timezone_ready (mm-iface-modem-time.c:218)
==226546== by 0x4EA827E: g_simple_async_result_complete (in /usr/lib/libgio-2.0.so.0.6600.7)
==226546== by 0x17AE2C: at_command_ready (mm-base-modem-at.c:538)
==226546== by 0x4EA827E: g_simple_async_result_complete (in /usr/lib/libgio-2.0.so.0.6600.7)
==226546== by 0x24B475: serial_command_ready (mm-port-serial-at.c:378)
==226546== by 0x4EA827E: g_simple_async_result_complete (in /usr/lib/libgio-2.0.so.0.6600.7)
==226546== by 0x244F94: command_context_complete_and_free (mm-port-serial.c:141)
==226546== by 0x246861: port_serial_got_response (mm-port-serial.c:755)
==226546== by 0x246923: port_serial_timed_out (mm-port-serial.c:785)
==226546== by 0x50863C3: ??? (in /usr/lib/libglib-2.0.so.0.6600.7)
==226546== by 0x5085B83: g_main_context_dispatch (in /usr/lib/libglib-2.0.so.0.6600.7)
|
|
|
|
mm-iface-modem-time.c: In function ‘interface_disabling_step’:
mm-iface-modem-time.c:484:18: error: this statement may fall through [-Werror=implicit-fallthrough=]
484 | ctx->step++;
| ~~~~~~~~~^~
mm-iface-modem-time.c:486:5: note: here
486 | case DISABLING_STEP_CANCEL_NETWORK_TIMEZONE_UPDATE:
| ^~~~
...
|
|
mm-iface-modem-time.c: In function ‘interface_disabling_step’:
mm-iface-modem-time.c:481:5: error: switch missing default case [-Werror=switch-default]
481 | switch (ctx->step) {
| ^~~~~~
...
|
|
|
|
|
|
If we're going to overwrite the timezone context, make sure everything
is cleaned up, including the signal handler. Otherwise, we may end up
running the signal handler without a context attached:
(gdb) bt
#0 network_timezone_state_changed (self=0x6aa1e8) at mm-iface-modem-time.c:266
#1 0x76a8d418 in g_closure_invoke (closure=0x6c9810, return_value=0x0, return_value@entry=0x1, n_param_values=2, param_values=0x7edb58c0, param_values@entry=0x6cdab8,
invocation_hint=invocation_hint@entry=0x7edb585c) at gclosure.c:804
#2 0x76a9e674 in signal_emit_unlocked_R (node=node@entry=0x6794d0, detail=0, detail@entry=391, instance=instance@entry=0x6aa1e8, emission_return=0x6794a8, emission_return@entry=0x0,
instance_and_params=instance_and_params@entry=0x7edb58c0) at gsignal.c:3629
#3 0x76aa31f4 in g_signal_emit_valist (instance=instance@entry=0x6aa1e8, signal_id=signal_id@entry=1, detail=detail@entry=6767564, var_args=..., var_args@entry=...) at gsignal.c:3385
#4 0x76aa34ec in g_signal_emit (instance=instance@entry=0x6aa1e8, signal_id=signal_id@entry=1, detail=391) at gsignal.c:3441
#5 0x76a91698 in g_object_dispatch_properties_changed (object=0x6aa1e8, n_pspecs=1, pspecs=<optimized out>) at gobject.c:1062
#6 0x76a90e7c in g_object_notify_queue_thaw (object=object@entry=0x6aa1e8, nqueue=nqueue@entry=0x75409418) at gobject.c:296
#7 0x76a9470c in g_object_set_valist (object=object@entry=0x6aa1e8, first_property_name=first_property_name@entry=0xad566 "iface-modem-state", var_args=..., var_args@entry=...) at gobject.c:2171
#8 0x76a94b18 in g_object_set (_object=_object@entry=0x6aa1e8, first_property_name=0xad566 "iface-modem-state") at gobject.c:2275
#9 0x0004b8a4 in __iface_modem_update_state_internal (self=0x6aa1e8, new_state=MM_MODEM_STATE_DISABLED, reason=MM_MODEM_STATE_CHANGE_REASON_USER_REQUESTED, failed_reason=MM_MODEM_STATE_FAILED_REASON_NONE)
at mm-iface-modem.c:1487
#10 0x0007047c in disabling_context_free (ctx=0x6ba850) at mm-broadband-modem.c:9053
#11 0x76b4e538 in g_task_finalize (object=0x682488) at gtask.c:636
#12 0x76a92800 in g_object_unref (_object=0x682488) at gobject.c:3183
#13 0x00072154 in iface_modem_disable_ready (self=0x6aa1e8, result=0x7011b0, task=0x682488) at mm-broadband-modem.c:9101
#14 0x76b4e6ec in g_task_return_now (task=0x7011b0) at gtask.c:1107
#15 0x76b4e720 in complete_in_idle_cb (task=0x7011b0) at gtask.c:1121
#16 0x7694d144 in g_main_dispatch (context=0x6765a8) at gmain.c:3154
#17 g_main_context_dispatch (context=context@entry=0x6765a8) at gmain.c:3769
#18 0x7694d40c in g_main_context_iterate (context=0x6765a8, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at gmain.c:3840
#19 0x7694d838 in g_main_loop_run (loop=0x6750d0) at gmain.c:4034
#20 0x0002bec0 in main (argc=<optimized out>, argv=<optimized out>) at main.c:181
|
|
The logic implementing the network timezone loading was holding a
strong reference to the modem (inside the GTask) while waiting to
be registered.
This was triggering some possible memory leaks as the modem object
could have been left around even after the modem was disconnected.
E.g. if the modem booted without antennas plugged in, it was never
getting registered, and if we unplugged it right away in that state,
the network timezone logic would have left a modem reference around
without disposing it.
This issue, combined with e.g. other interfaces relying on disposing
its own logic with the last object reference (e.g. Signal interface
sets up its logic in a context bound to the lifetime of the object)
would mean that a lot of different logic blocks were kept running even
after the modem device was unplugged from the system.
We avoid this issue by making sure that no new additional reference is
taken by the logic in charge of updating the network timezone. We just
make the timezone update context be bound to the lifetime of the
object, as other interfaces do.
While doing this, we also remove the logic in charge of "cancelling"
the context, as it isn't needed. If the logic needs to be cancelled,
we would just remove any configured timeout, which is enough.
As part of the changes, the logic has also been improved so that the
network timezone isn't only updated the first time the modem gets
registered. If the modem gets unregistered and re-registered, we would
reload the network timezone information.
|
|
Commit 7c5f308af ("iface-modem-time: port update_network_timezone to use
GTask") ported most of the update_network_timezone code to use GTask,
but missed a g_simple_async_report_error_in_idle call, which could
potentially lead to an incorrect G_TASK cast.
|
|
|
|
|
|
|
|
|
|
g_free and g_object_unref are in form of `void (*)(gpointer)`, which
matches the GDestroyNotify signature. An explicit GDestroyNotify cast on
g_free and g_object_unref is thus not needed.
|
|
|
|
The interfaces usually retrieve objects (e.g. skeletons) from the Modem object
using g_object_get(), but we didn't make sure that these objects were actually
valid before using them.
This should clean up errors happening when the modem gets unplugged and still
some actions are ongoing.
Should fix https://bugzilla.gnome.org/show_bug.cgi?id=685933
|
|
Both the ModemManager daemon and the mmcli will now include `libmm-glib.h' only.
We also handle two new special `_LIBMM_INSIDE_MM' and `LIBMM_INSIDE_MMCLI'
symbols, which if included before the `libmm-glib.h' library allow us to:
* Don't include the libmm-glib high level API in the ModemManager daemon, as
the object names would clash with those in the core.
* Define some of the methods of helper objects to be included only if compiling
ModemManager daemon or the mmcli.
|
|
|
|
* mm_base_modem_peek_port_* () will return either a port object (no new
reference), or NULL if none available.
You would usually peek() a port if you're going to use it just in the current
method, as there is no way to that reference to get invalid (we're single
threaded).
* mm_base_modem_get_port_* () will return either NEW references to valid
port objects, or NULL if none available.
And, you would usually get() a port, whenever you want the port object to be
valid even out of the current method, for example when keeping it in the
context of an async operation.
Also, we need to consider that the primary AT port MAY BE NULL when you
peek() or get() it. This is due to the fact that we may be releasing ports
(due to device disconnection) in the middle of async operations.
|
|
Modems may get notified via unsolicited messages (either AT or QMI or some
other thing), that the network time was updated. This setup allows subclasses
implementing the Time interface to setup/cleanup and enable/disable the
unsolicited messages, as well as notify about the network time updates.
|
|
|
|
We don't want to retry if the error is fatal in some other kind, e.g. a reply
parsing error or something.
|
|
Following the same logic as in the original implementation, we try to load the
network timezone information only after being registered, and also with up to N
retries. The async operation in charge of the loading can be cancelled
gracefully, i.e. if the interface is disabled before we get ever registered.
|
|
|