Skip to content

[th/libnm-deprecate-sync-api] libnm: deprecate synchronous/blocking API in libnm

Thomas Haller requested to merge th/libnm-deprecate-sync-api into master

Quote commit message:

Note that D-Bus is fundamentally asynchronous. Doing blocking calls
on top of that is wrong, especially for libnm. That is because libnm
also provides a client-side cache of the objects of the D-Bus
interface. This cache is filled by asynchronous D-Bus events (of course).
So, making a blocking D-Bus call means to wait for a response and
return it, while queuing everything that happens in between. The queued
messages will be queued in the GMainContext and only be processed after
the context gets iterated. That violates the strict ordering that we want
to guarantee. It also means, when the blocking call returns, then the cache
is still frozen in the state as it was before the call.

Read also [1], for why blocking calls are wrong.

[1] https://smcv.pseudorandom.co.uk/2008/11/nonblocking/

Mark all this API as deprecated. Also, this serves the purpose of
identifying blocking D-Bus calls in libnm.

Note that API like nm_device_wifi_request_scan_options() indeed
does not really need to be in sync with the platform cache. I mean,
it makes a blocking request to start scanning, which does not change
the content of the cache and there wouldn't be a severe problem with
this API. However, it requires as source-argument a NMDeviceWifi, instead
of a plain D-Bus path. If that API would be just a convenience wrapper around
g_dbus_connection_call_sync(), and accept as target a plan D-Bus path,
having such API would be fine. But having synchronous API that operates on a
NMDeviceWifi (an object from the D-Bus cache) is wrong because they D-Bus
messages are handled out of order with the rest of the messages that are relevant
to NMClient. Hence deprecate such functions too.

The only other remainging API is the synchronous GInitable call for
NMClient. That is an entirely separate beast and of course wrong too.
Note that synchronous API in NMSecretAgentOld, NMVpnPluginOld and
NMVpnServicePlugin as also not deprecated. These types are not part
of the D-Bus cache.

Note that "deprecated" here does not really mean that they API is going
to be removed. We don't break API. Just that it's awkward and
discouraged.

Merge request reports