Code review comment for lp:~pitti/indicator-session/libupower-glib

Ted Gould wrote:
> On Tue, 2010-02-16 at 15:28 +0000, Martin Pitt wrote:
>
>> Review: Resubmit
>>
>>> It uses the _sync calls, but we have a requirement to make all DBus
>>> operations async in the various indicator code. I'm unclear whether
>>> up_client_get_can_suspend is sync or async.
>>>
>> It gets the property synchronously on the first call (i. e. during
>> initialization of indicator-session, and then just returns the cached
>> value. I. e. in the "changed" signal handler the property values are
>> already in the UpClient object's cache and no D-Bus communication
>> needs to happen at all.
>>
>
> So I went and grabbed the upower code and basically none of this is
> Async. At all. I'd love to use the libupower library so that we're in
> a better place with the interface, but man:
>
> ted@shi:~/Packaging/upower/upower-0.9.0+git20100216.b9bb78$ rgrep _async
> *
> src/up-daemon.c: ret = g_spawn_command_line_async (command, &error);
> ted@shi:~/Packaging/upower/upower-0.9.0+git20100216.b9bb78$
>
Ouch.
> I'm not sure what to do here. David, do you have opinion? I'd hate to
> rewrite the PolicyKit code, but we've made it a core architectural point
> to only use async DBus. I doubt we could patch libupower at this point
> as it'd be a pretty big patch.
>
Well, I guess then it means either keeping your async code, and...
ugh... adding the PK per-user policy bits (no idea how complex that can
be), or...

...adding a thread where calls to upower can block. The lib is not
advertised as being thread safe, but I haven't spotted big static
variables in the library source, so that may reasonably work.

David