core: convert NMDBusObject's "path" property to signal "exported-changed"

The GObject property "path" exists for the sole reasons so that
other components can connect to the "notify::path" signal.

However, notifications are blocked by g_object_freeze_notify(),
and especially for NMDBusObject we want to make use of that to
combine multiple PropertiesChanged events into one.

This blocking of the signal is not desired for the case where
we wait for "notify::path". Convert that to a signal, which
will not be blocked.
This commit is contained in:
Thomas Haller
2018-03-29 19:27:24 +02:00
parent 7595b4f1c7
commit cd71ec3084
5 changed files with 77 additions and 38 deletions

View File

@@ -9577,8 +9577,7 @@ nm_device_activate_ip6_state_done (NMDevice *self)
/*****************************************************************************/
static void
act_request_set_cb (NMActRequest *act_request,
GParamSpec *pspec,
act_request_exported_changed (NMActRequest *act_request,
NMDevice *self)
{
_notify (self, PROP_ACTIVE_CONNECTION);
@@ -9610,8 +9609,8 @@ act_request_set (NMDevice *self, NMActRequest *act_request)
if (act_request) {
priv->act_request_id = g_signal_connect (act_request,
"notify::"NM_DBUS_OBJECT_PATH,
G_CALLBACK (act_request_set_cb),
NM_DBUS_OBJECT_EXPORTED_CHANGED,
G_CALLBACK (act_request_exported_changed),
self);
switch (nm_active_connection_get_activation_type (NM_ACTIVE_CONNECTION (act_request))) {
@@ -15220,7 +15219,7 @@ get_property (GObject *object, guint prop_id,
g_variant_new ("(uu)", priv->state, priv->state_reason));
break;
case PROP_ACTIVE_CONNECTION:
nm_dbus_utils_g_value_set_object_path (value, priv->act_request_public ? priv->act_request : NULL);
nm_dbus_utils_g_value_set_object_path_still_exported (value, priv->act_request_public ? priv->act_request : NULL);
break;
case PROP_DEVICE_TYPE:
g_value_set_uint (value, priv->type);

View File

@@ -37,9 +37,13 @@ nm_dbus_object_set_quitting (void)
/*****************************************************************************/
NM_GOBJECT_PROPERTIES_DEFINE (NMDBusObject,
PROP_PATH,
);
enum {
EXPORTED_CHANGED,
LAST_SIGNAL,
};
static guint signals[LAST_SIGNAL] = { 0 };
G_DEFINE_ABSTRACT_TYPE (NMDBusObject, nm_dbus_object, G_TYPE_OBJECT);
@@ -53,6 +57,12 @@ G_DEFINE_ABSTRACT_TYPE (NMDBusObject, nm_dbus_object, G_TYPE_OBJECT);
/*****************************************************************************/
static void
_emit_exported_changed (NMDBusObject *self)
{
g_signal_emit (self, signals[EXPORTED_CHANGED], 0);
}
static char *
_create_export_path (NMDBusObjectClass *klass)
{
@@ -107,6 +117,8 @@ nm_dbus_object_export (NMDBusObject *self)
g_return_val_if_fail (!self->internal.path, self->internal.path);
nm_assert (!self->internal.is_unexporting);
self->internal.path = _create_export_path (NM_DBUS_OBJECT_GET_CLASS (self));
self->internal.export_version_id = ++id_counter;
@@ -115,7 +127,7 @@ nm_dbus_object_export (NMDBusObject *self)
_nm_dbus_manager_obj_export (self);
_notify (self, PROP_PATH);
_emit_exported_changed (self);
return self->internal.path;
}
@@ -135,12 +147,27 @@ nm_dbus_object_unexport (NMDBusObject *self)
_LOGT ("unexport: \"%s\"", self->internal.path);
/* note that we emit the signal *before* actually unexporting the object.
* The reason is, that listeners want to use this signal to know that
* the object goes away, and clear their D-Bus path to this object.
*
* But this must happen before we actually unregister the object, so
* that we first emit a D-Bus signal that other objects no longer
* reference this object, before finally unregistering the object itself.
*
* The inconvenient part is, that at this point nm_dbus_object_get_path()
* still returns the path. So, the callee needs to handle that. Possibly
* by using "nm_dbus_object_get_path_still_exported()". */
self->internal.is_unexporting = TRUE;
_emit_exported_changed (self);
_nm_dbus_manager_obj_unexport (self);
g_clear_pointer (&self->internal.path, g_free);
self->internal.export_version_id = 0;
_notify (self, PROP_PATH);
self->internal.is_unexporting = FALSE;
}
/*****************************************************************************/
@@ -204,22 +231,6 @@ nm_dbus_object_emit_signal (NMDBusObject *self,
/*****************************************************************************/
static void
get_property (GObject *object, guint prop_id,
GValue *value, GParamSpec *pspec)
{
NMDBusObject *self = NM_DBUS_OBJECT (object);
switch (prop_id) {
case PROP_PATH:
g_value_set_string (value, self->internal.path);
break;
default:
G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
break;
}
}
static void
dispatch_properties_changed (GObject *object,
guint n_pspecs,
@@ -284,10 +295,11 @@ dispose (GObject *object)
g_warn_if_reached ();
nm_dbus_object_unexport (self);
}
} else if (nm_clear_g_free (&self->internal.path)) {
} else if (self->internal.path) {
/* FIXME: do a proper, coordinate shutdown, so that no objects stay
* alive nor exported. */
_notify (self, PROP_PATH);
_emit_exported_changed (self);
nm_clear_g_free (&self->internal.path);
}
G_OBJECT_CLASS (nm_dbus_object_parent_class)->dispose (object);
@@ -302,14 +314,13 @@ nm_dbus_object_class_init (NMDBusObjectClass *klass)
object_class->constructed = constructed;
object_class->dispose = dispose;
object_class->get_property = get_property;
object_class->dispatch_properties_changed = dispatch_properties_changed;
obj_properties[PROP_PATH] =
g_param_spec_string (NM_DBUS_OBJECT_PATH, "", "",
NULL,
G_PARAM_READABLE |
G_PARAM_STATIC_STRINGS);
g_object_class_install_properties (object_class, _PROPERTY_ENUMS_LAST, obj_properties);
signals[EXPORTED_CHANGED] =
g_signal_new (NM_DBUS_OBJECT_EXPORTED_CHANGED,
G_OBJECT_CLASS_TYPE (object_class),
G_SIGNAL_RUN_FIRST,
0, NULL, NULL,
g_cclosure_marshal_VOID__VOID,
G_TYPE_NONE, 0);
}

View File

@@ -82,7 +82,7 @@ extern const NMDBusInterfaceInfoExtended nm_interface_info_device_statistics;
#define NM_IS_DBUS_OBJECT_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), NM_TYPE_DBUS_OBJECT))
#define NM_DBUS_OBJECT_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), NM_TYPE_DBUS_OBJECT, NMDBusObjectClass))
#define NM_DBUS_OBJECT_PATH "path"
#define NM_DBUS_OBJECT_EXPORTED_CHANGED "exported-changed"
/* NMDBusObject and NMDBusManager cooperate strongly. Hence, there is an
* internal data structure attached to the NMDBusObject accessible to both of them. */
@@ -99,6 +99,7 @@ struct _NMDBusObjectInternal {
* unexported, or even re-exported afterwards. If that happens, we want
* to fail the request. For that, we keep track of a version id. */
guint64 export_version_id;
bool is_unexporting:1;
};
struct _NMDBusObject {
@@ -169,6 +170,18 @@ nm_dbus_object_is_exported (NMDBusObject *self)
return !!nm_dbus_object_get_path (self);
}
static inline const char *
nm_dbus_object_get_path_still_exported (NMDBusObject *self)
{
g_return_val_if_fail (NM_IS_DBUS_OBJECT (self), NULL);
/* like nm_dbus_object_get_path(), however, while unexporting
* (exported-changed signal), returns %NULL instead of the path. */
return self->internal.is_unexporting
? NULL
: self->internal.path;
}
const char *nm_dbus_object_export (NMDBusObject *self);
void nm_dbus_object_unexport (NMDBusObject *self);

View File

@@ -120,6 +120,20 @@ nm_dbus_utils_g_value_set_object_path (GValue *value, gpointer object)
g_value_set_string (value, "/");
}
void
nm_dbus_utils_g_value_set_object_path_still_exported (GValue *value, gpointer object)
{
const char *path;
g_return_if_fail (!object || NM_IS_DBUS_OBJECT (object));
if ( object
&& (path = nm_dbus_object_get_path_still_exported (object)))
g_value_set_string (value, path);
else
g_value_set_string (value, "/");
}
void
nm_dbus_utils_g_value_set_object_path_from_hash (GValue *value,
GHashTable *hash /* has keys of NMDBusObject type. */,

View File

@@ -173,6 +173,8 @@ const char **nm_dbus_utils_get_paths_for_clist (const struct CList *lst_head,
void nm_dbus_utils_g_value_set_object_path (GValue *value, gpointer object);
void nm_dbus_utils_g_value_set_object_path_still_exported (GValue *value, gpointer object);
void nm_dbus_utils_g_value_set_object_path_from_hash (GValue *value,
GHashTable *hash,
gboolean expect_all_exported);