Possible races when using lenses

[Michal Hruby]

There are currently possible races when using lenses - currently a lens will emit the "Changed" signal once it starts up which basically contains all properties of a lens - that means there's time between getting an active GDBusProxy and receiving the "Changed" signal when things can go wrong.

To fix this issue we should be using dbus properties which are automatically read when getting a proxy and readily available after g_dbus_proxy_new_finish call. This will also allow us to get rid of the InfoRequest() method.

[Mikkel Kamstrup Erlandsen (kamstrup)]

I don't see how the two solutions differ. They both require an async dbus call to fully set up the proxy... InfoRequest() or GetProperties(), sure one is automated by gdbus, but breaking wire protocol just for that seems like a steep price for no real gain..?

[Michal Hruby (mhr3)]

They do differ, because as you said GetAll() is automatically done by gdbus (even now, so there'd be fewer context switches!), so we get rid of the time where a lens proxy is half-initialized ie connected but not knowing if it should search in home view, is visible, etc.

Moreover we don't need to break the wire protocol, the InfoRequest method can stay, but we won't use it.

[Mikkel Kamstrup Erlandsen (kamstrup)]

Oh, now I get it. I was expecting InfoRequest() to return the same values as passed in the Changed() signal.

But basically InfoRequest() is just a dummy to DBus activate the lens I guess. We could probably just have used a standard DBus Peer.Ping() message for that in stead.

IIRC the reason we have it like this is because we didn't want to force the lenses to populate all properties right on the spot when they were activated. Unity can pre-render a dummy with the info from the .lens file. This is to relieve some IO stress on startup. By having a signal as the only mechanism for this the lenses have some wiggle room in how they start up.

AND! If we're really keen on shaving off context switches I'd rather drop the idea of subscribing to Changed() on each lens proxy, but simply have one single DBus match rule to catch them all (just like pokemon - you gotta catch them all!). Ie. AddMatch("type='signal',interface='com.canonical.Unity.Lens',member='Changed'")

[Michal Hruby (mhr3)]

> IIRC the reason we have it like this is because we didn't want to force the lenses to populate all properties right on the spot when they were activated.

That would be pretty dangerous - it'd imply that lenses are fully operational only after a InfoRequest() call, but because of dbus-activation the first method call can easily be Search() or even Activate(uri).

> AND! If we're really keen on shaving off context switches I'd rather drop the idea of subscribing to Changed() on each lens proxy, but simply have one single DBus match rule to catch them all (just like pokemon - you gotta catch them all!). Ie. AddMatch("type='signal',interface='com.canonical.Unity.Lens',member='Changed'")

Using standard dbus-properties and their change notification signal would probably fix this automagically.

[Michal Hruby (mhr3)]

> Using standard dbus-properties and their change notification signal would probably fix this automagically.

But ultimately the Changed signal isn't really needed in most cases, most lenses won't change any of the properties after initially setting them.