Re: Calling async functions of NMClient from another thread

[Thomas Haller <thaller redhat com>]

On Sat, 2021-10-09 at 14:05 +0200, Luis Felipe via networkmanager-list
wrote:
> Hi all,
>
> it's my first post, so please apologize for any mistake.
>
> I have a trouble with NMClient (libnm) and threads in my C-application.
> Here the scenario:
>
> - in the main thread, gmain-loop and NMClient are created; here
> 'g_main_loop_run()' is called too
>
> - in a separated thread, a XML-RPC server is started. Each time a new
> RPC arrives, this server creates a new thread with the proper function
>
> - the RPC-function calls an async NMClient-function
> (nm_client_activate_connection_async), and the following errors appear:
>
> <error>
> (process:668): GLib-CRITICAL **: 17:53:29.664:
> g_main_context_push_thread_default: assertion 'acquired_context
> ' failed
>
> (process:668): GLib-CRITICAL **: 17:53:29.664:
> g_main_context_pop_thread_default: assertion 'g_queue_peek_head
>  (stack) == context' failed
> </error>
>
> I expected that the async-functions can be called from any context, but
> doesn't look like, or am I doing something wrong?
> Please can somebody give me a hint for solving this issue? Thanks in
> advance.
>
> BR,


hi,


First of all, NMClient is a cache of the NetworkManager data you see on
the D-Bus API. Accessing that cache is not thread safe. You you need to
coordinate access. This does NOT work via some mutex. Instead you need
"acquire" exclusive access to the GMainContext (which is associated
with the NMClient instance: nm_client_get_main_context()). 

That is, because as nm_client_get_main_context() gets iterated, D-Bus
events and timeout events may be processed, and the NMClient instance
may change at any time. So to ensure that the NMClient instance is
unchanging, you need to ensure that nobody else is currently iterating
the context. Technically, when accessing NMClient you don't need to
acquire the GMainContext on your current thread, it would be enough
that no other thread has it acquired. If another thread has it
acquired, there are races because the data might change.

In particular, running the context on the main thread with
g_main_loop_run() means you must not access NMClient (aside
g_object_ref/g_object_unref and accessing a subset of trivial functions
like nm_client_get_dbus_connection() and nm_client_get_main_context() -
- but not for example nm_client_get_dbus_name_owner()).


Indeed, nm_client_activate_connection_async() uses GTask and does
little more than calling g_dbus_connection_call()... except, before
making the D-Bus call it does 

  dbus_context = nm_g_main_context_push_thread_default_if_necessary(priv->dbus_context);

(see [1]). The reason is that the D-Bus reply must first be processed
on nm_client_get_main_context() so that the cache can be updated with
the new data, and that the response is in order with the other D-Bus
signals we process, then GTask will make that the response gets
dispatched on the callers main-context. So the problem is really that
nm_client_get_main_context() is already acquired on the main thread and
g_main_context_push_thread_default() fails.

[1] 
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/blob/06d448245b26709d6eb5c24279c352cb17df7cdf/src/libnm-client-impl/nm-client.c#L535

Instead, you need to coordinate access to the NMClient instance by
passing access to the GMainContext around. You might iterate the
g_main_context_default() on the main thread, and give NMClient a
*different* GMainContext instance. The problem then is that you still
need to take care that NMClient's context gets iterated at the right
times (but also that it gets iterated *all* the time). That
unfortunately seems non trivial, also because glib does not provide a
function to integrate a GMainContext in another GMainContext. In libnm,
we do that this way: [2].

*************************
TL;DR: THE RIGHT SOLUTION: So the best solution might be to schedule
any access via g_idle_source_new() on NMClient's GMainContext.

Another theoretical solution is that you just make the D-Bus call
ActivateConnection yourself, with g_dbus_connection_call(). Of course,
then the response is again not in sync/ordered with respect to the
events/state of NMClient, which brings other problems. And you still
couldn't access NMClient directly from the other thread.


Btw, before version 1.22, you could *only* use NMClient with
g_main_context_default(). Try not to use such an old libnm version.

[2] 
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/blob/06d448245b26709d6eb5c24279c352cb17df7cdf/src/libnm-glib-aux/nm-shared-utils.c#L5488

best,
Thomas