outside-dispatch-lock handlers

[Thiago Macieira]

[Trying to keep my reply to a minimum size...]

Havoc Pennington wrote:
>Let's define "processing the handler queue" as:
> - for each handler in the queue that applies to the same
>   message, run the handler
> - if a handler returns NEED_MEMORY, put it back in
>   the queue and return from dbus_connection_dispatch()
>   (this is how you can get handlers for multiple distinct
>   messages in the queue at the same time)
> - if a handler returns HANDLED, drop the rest of the queue
>   that applies to the same message, and return
> - if a handler returns NOT_YET_HANDLED, remove the handler
>   and run the next handler
>
>The overall dbus_connection_dispatch() algorithm then is:
> - if the handler queue is not empty, process the handler
>   queue and return. This runs handlers for at most one
>   message, possibly leaving handlers for more messages
>   in the queue.
> - if the handler queue is empty, and the message queue
>   is not empty, borrow the next message (take dispatch lock)
> - add all handlers for that message to the handler queue;
>   if we run out of memory here, remove handlers and
>   return the message and return from dispatch
>   (i.e. fail atomically)
> - once we've queued all handlers, steal the message
>   (drop dispatch lock)
> - process the handler queue as defined earlier

IIUC, this means "process the handler queue without a dispatch lock". Is 
that so? That sounds great for me. :-)

>OK so say we have signals A, B, C, with handlers:
>
>A: 1, 2, 3
>B: 4, 5, 6
>C: 7, 8, 9
>
>while dispatching A, inside handler 2 we dispatch again twice; we'd
>first run handler 3, and then queue and run 4, 5, 6.

I'd rather 3 weren't run until 2 finished running.

>But the problem is that we don't know whether we should run 3 until we
>know the return value of 2. i.e. right now there's a requirement that we
>always _complete_ each handler before we can run the next.

Exactly.

>The "outside handler" idea essentially splits each handler into two
>halves:
> - the "will I handle this?" half
> - the "actually handle this" half
>
>If we did that, we could run the "will I handle this?" half when we
>build the handler queue, and the "actually handle this" half could be
>run when we process the handler queue. This means that in the above
>example, we would know whether 3 should be run or not, before 2 has
>returned.

"we would know whether the 'actually handle 3' would be run 
before 'actually handle 2' has returned" <-- is that what you meant?

>One way to think of this is that we're keeping all the state of
>dbus_connection_dispatch() in the DBusConnection instead of local
>variables. Whenever you call dbus_connection_dispatch(), it just
>"resumes" where it was last.

Hmm... not sure I agree. I've got to think a bit more about the 
consequences, but I'd rather dispatching were "atomic". I.e., either 
dispatch() does it fully or it does not. And recursively calling 
dispatch() won't resume an earlier dispatch -- it can only start a new 
one.

>I think the effect of all this is that when a handler recurses, it
>"jumps itself ahead" in the handler queue. i.e. if I'm handler 2, when I
>recurse any number of the other handlers 3-9 could be called.

Yes. If you recurse, you *want* other handlers to be called.

For instance, you could recurse in order to wait for a signal that you 
know is coming. (Hal sends 3 signals PropertyChanged if you try to lock a 
device)

>When the 
>recursive dispatch returns, then handler 2 would be running _after_
>handler 9. However, a handler does not jump _other_ handlers out of
>order - handler 3 still runs before 4.

I wouldn't say "after 9", but "handler 2 is running _around_ handlers 
4-9". I still think handler 3 should be run after 2 has finished, since 
it's processing the same message.

>Say you are crazy and now you start calling dispatch() from several
>threads at once... first theory is "don't do that then" but say we
>wanted to invent something 'sane' to happen, my best suggestion is:
> - we hold a recursive lock when invoking a handler
> - that handler can recurse normally, other threads wait
>this preserves ordering of handlers and still allows recursive dispatch.

Agreed. Should this be done in libdbus or in the binding? If in libdbus, 
we need to add a new lock and add recursive mutex behaviour.

But going back to your A, B, C & 1-9 example, if thread α called 
dispatch() to handle message A and, before it finished, thread β called 
dispatch() too, why not let it handle message B?

Of course, if the binding code is doing that, ALL handlers must be not 
only reentrant, they must be thread-safe. Which again poses the question 
of cooperation between bindings: are everyone's handlers thread-safe?

>I think this is normally a lot safer and nicer for apps than running all
>the intermediate handlers up to the message reply, if they just want to
>block for the message reply. But if you want to also process incoming
>method calls etc. while blocking you would just avoid ever using
>send_with_reply_and_block() from your current thread, only use
>dispatch() or pop().

That's what I'm doing.

>> Which brings us back to the problem of self-thread recursing. If the
>> user code recurses and wants to process more messages, there can only
>> be two possible outcomes: either we find the specific message that the
>> user wants to process among the many messages we're receiving, or we
>> process them all.
>>
>> I chose solution #2 for the Qt bindings for the moment. DCOP's
>> solution is #1.
>
>If I understand you correctly, my opinion is that solution #2 is right
>for method calls and signals, and solution #1 is right for method
>replies. Note that send_with_reply_and_block()/DBusPendingCall implement
>solution #1 for method replies. Solution #2 I'm theorizing should work
>as I just described above (the "handler queue" model)

The difference here is the concept of "find the specific message the user 
wants". DCOP understands that a method call coming from the same app that 
you called while it's processing your call is something you want to 
handle. More so, if you're calling app A, A called B during the 
processing of your call, and B called you back while processing A's call, 
DCOP also understands that you need to process the call.

Extend that to signals as well in D-Bus.

>The wait condition is for an IO lock on the socket, which only needs to
>be acquired to read/write from the socket. So it takes the IO lock in
>order to then block on the socket. If several threads try to
>send_with_reply_and_block, then one will block on the socket and the
>others will block on the IO lock until they get woken up. The dispatch()
>thread also participates in this, so whenever it needs to block it can
>block either on the socket or on the IO lock. All these threads will be
>queuing up messages as they read from the socket, but only the dispatch
>thread will drain the queue - the send_with_reply_and_block threads will
>just pull out the exact message they are waiting for, and otherwise
>ignore everything.

Is there any possibility that a dispatch() running at the same time as 
send_with_reply_and_block steal the message the block is waiting for?

>Anyway, considering all this what concrete changes would be needed to
>implement the "handler queue" semantics:
> - we need each handler to have both "will I handle?" and
>   "actually handle" operations (ideally not provided by the app
>   programmer - bindings should be able to do these most of the time)

Indeed. I'd advise against calling any user code from the "will I handle?" 
operation.

> - when calling the "will I handle?", recursion is not allowed/sensible,
>   a non-recursive lock would be fine around these calls
> - when calling the "actually handle", recursion is allowed in the same
>   thread, a recursive lock would be used around these calls
>
>We would have to work out the specific API changes to get the two
>operations for each handler, or some clever way to avoid these changes.
>
>The HANDLED_OUTSIDE_DISPATCH change effectively does add the two
>operations, though in a sort of strange way.

Yep. For a first draft, it was a good idea, but it needs refining.

>The current handler function becomes "will I handle?" and it returns
>HANDLED_OUTSIDE_DISPATCH if yes, and NOT_YET_HANDLED if no.
>If yes, then no further handlers are added to the handler queue for the
>message, if no then this handler is added but others may be also.
>This function can't recursively dispatch.

It currently can't, so there's no problem.

>A "legacy" handler not aware of the new setup will behave a bit oddly,
>since it will "actually handle" when asked "will you handle?" - this
>means it will be ordered incorrectly with respect to handlers in new
>code that get postponed until the handler queue is processed, and it
>means that recursion won't be allowed inside a "legacy" handler.

It already isn't allowed. No change here.

>If we ignored back compat, we could probably make this a lot less
>confusing. Something like:
>
> enum HandlerCheckResult {
>    INVOKE_HANDLER,
>    INVOKE_HANDLER_THEN_STOP_PROCESSING,
>    DO_NOT_INVOKE_HANDLER,
>    NEED_MEMORY
> }

We could do that by adding the new enum values as copies of the current 
ones, then drop compat in a later version.

-- 
Thiago José Macieira - thiago.macieira AT trolltech.com
Trolltech AS - Sandakerveien 116, NO-0402 Oslo, Norway
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 191 bytes
Desc: not available
Url : http://lists.freedesktop.org/archives/dbus/attachments/20060302/8966f280/attachment-0001.pgp