C convenience lib (was Re: Is dbus_pending_call_set_notify() thread safe?)

[Havoc Pennington]

Hi,

Fan Wu wrote:
> What I'm thinking it's acutally a variant of your 1)b. The only
> difference is the threading part is hidden in the API.

Here's what I'm confused about. You still have to expose some mechanism 
for async to the app, OR you have to block the app. Meaning, you can't 
really hide the threading part.

The app can see either:
  - async where threads are visible to the app, i.e. app gets method call
    reply in a different thread from main thread
  - async where it assumes some main loop but not a particular one; the
    app gets a descriptor it has to watch, then when there's stuff on
    that descriptor the app wakes up the library
  - not async, the library just blocks on the dbus descriptor

So libdbus already offers all three of those models. It might could have 
a somewhat different API or more convenient API, but no matter what the 
API, the model will be one of the above.

The fourth model is:
  - async where the library knows the main loop, so rather than giving
    the app descriptors and timeouts, the library can just add the
    descriptors and timeouts to the main loop directly

So the fourth model is the only convenient/easy-to-use async model, but 
requires the library to know the main loop, which is why libdbus does 
not offer this model directly.

> So pseudo code
> would be like this:
> 
> //this will spawn a worker thread in the background
> // which will try to connect to the bus,
> // try to hold a name etc
> dbus_convenience_api_initialize(bus_name,
>                                 callbacks,
>                                 name_to_own...);
> 
> //in the main thread
> while(continue_looping) // or poll/select/WaitforMultipleObjects
> {
>    ...
>    if(got_stuff_to_send)
>    {
>       //this is the part that needs to wake up worker thread
>       //in case it's in poll/select sleep
>       dbus_convenience_api_send(conn, msg);
>    }
>    ...
> }
> 
> //wake up the worker thread and exit it
> dbus_convenience_api_finalize(handle)

We could have an API like this but what I'd point out is that the 
callbacks all have to be invoked in the worker thread. That would mean 
that the app "sees" the worker thread; it isn't transparent. The 
callbacks would either have to do all their work in the worker thread 
(which would mean the whole app has to be thread safe), or the callbacks 
would have to "send" work back to the main thread - which would involve 
adding a file descriptor that the main thread would have to watch.

In other words nothing about threading or main loops is substantively 
changed here; all the tradeoffs are still the same as I described in the 
first part of this mail.

I wonder if what you're mainly getting at is just a bug.

Right now, if your DBusConnection is dispatched by a main loop you can 
set a "wakeup main loop" function, which is called e.g. in 
dbus-connection.c:_dbus_connection_send_preallocated_unlocked_no_update()

If you have a main thread that dispatches the connection from a main 
loop, and you dbus_connection_send(), then the wakeup main function will 
"kick" the main loop and the main loop can then write out the message 
you want to send.

However, if you don't have a main loop set up, but instead are using 
dbus_pending_call_block() or send_with_reply_and_block() or 
dbus_connection_read_write_dispatch(), then there is no wakeup mechanism.

So what we could do is, whenever we currently call wakeup_main_function, 
we could *also* wakeup our own poll() if we are currently blocking, 
using an extra pipe as you described.

This seems like a serious bug if you 1) want to send messages from 
multiple threads and 2) are just using dbus blocking calls or 
dbus_connection_read_write_dispatch() rather than setting up a main loop 
on the connection.

Is that the situation you are in?

I'm not sure if this is the bug you are encountering, though?

> The solution I'm thinking about is, add a wakeup handle (unamed
> pipe/socketpair for Unix, unamed event for windows) to the transport,
> then in socket_do_iteration() both the data socket and the wakeup
> handle get polled. If the data socket is blocking, the sleep can be
> interrupted through the wakeup handle. The next question is the timing
> of using the wakeup handle. The socket is programmed to block
> infinitely (unless it's waiting for a reply), so if there is only one
> sending thread, the sender shall be able to use the wakeup trick
> whenever it fails to acquire the IO path. If there are multiple sender
> threads (it's possible but not recommended by the model above), it
> shall not be a problem by wakeup worker thread multiple times. The
> worst case is the worker wakes up and find outgoing queue is empty. In
> this case it shall go back to blocking again.
> 
> This wakeup feature is intended for client only.
> 
> Thoughts?

This seems like a reasonable bug fix, however I think it's just an 
internal bug fix not necessarily new API - basically we need a 
wakeup_main_function for the poll() dbus itself does when you block 
instead of using the application's main loop.

New API seems like it would be a separate/orthogonal kind of discussion, 
right?

Havoc