Hi Luiz, On Tue, May 3, 2016 at 12:25 PM, Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx> wrote: > Hi Vinicius, > > On Tue, May 3, 2016 at 5:56 PM, Vinicius Costa Gomes <vcgomes@xxxxxxxxx> wrote: >> Consider the following example: >> >> /foo >> properties: "A", "B" >> >> /bar >> properties: "C", "D" >> >> If during a given mainloop iteration, property "A" of object '/foo' is >> changed, then properties "C" and "D" of '/bar', lastly "B" of '/foo', >> the current code will emit the PropertiesChanged signals in following >> order: "A", "B", "C", "D". >> >> This may confuse applications that have a dependency on the order of >> those signals. >> >> This fixes the ordering, so in the example, the order becomes: >> "C", "D", "A", B". This is considered not to be a problem, as >> applications may use the flag G_DBUS_PROPERTY_CHANGED_FLAG_FLUSH, so >> property changed signals are emitted as soon as possible. >> >> The solution is for each object, to reschedule the signals everytime a >> signal is emitted. > > So any changes actually pushes the object to last in the idle queue. I > guess this is fine provided we don't have more problems with this > ordering, this is a bit unfortunate side effect of grouping it seems, > otherwise we would have to emit several more signals to keep the order > the same. We might also keep some documentation about this design > choice though, making clear that in case the object update shall not > be put to last then G_DBUS_PROPERTY_CHANGED_FLAG_FLUSH shall be used. Cool. Will update the documentation then. > >> --- >> gdbus/object.c | 9 +++++++-- >> 1 file changed, 7 insertions(+), 2 deletions(-) >> >> diff --git a/gdbus/object.c b/gdbus/object.c >> index a220101..fc94e5e 100644 >> --- a/gdbus/object.c >> +++ b/gdbus/object.c >> @@ -635,11 +635,16 @@ static gboolean g_dbus_args_have_signature(const GDBusArgInfo *args, >> >> static void add_pending(struct generic_data *data) >> { >> - if (data->process_id > 0) >> - return; >> + guint old_id = data->process_id; > > Actually why didn't you just replace the return with > g_source_remove(data->process_id)? I guess that should be enough to > force it last since g_idle_add should probably be using a list and > appending the data it should be enough to move it back after others. If I understand your idea correctly, I may end up with multiple copies of the same element in the pending list (I did try that initially), and I don't think that the code is (nor should be) ready to handle that. > >> data->process_id = g_idle_add(process_changes, data); >> >> + if (old_id > 0) { >> + /* If there was an old idler, remove it. */ >> + g_source_remove(old_id); >> + return; >> + } >> + >> pending = g_slist_append(pending, data); >> } >> >> -- >> 2.8.1 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in >> the body of a message to majordomo@xxxxxxxxxxxxxxx >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > -- > Luiz Augusto von Dentz Cheers, -- Vinicius -- To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html