Re: implementing IN_PROGRESS

Havoc Pennington wrote:
>>      calls dbus_connection_dispatch (TAKES DISPATCH LOCK)
>>        acquire locks
>>        for each message in connection queue:
>
>I see. This is the key - right now dbus_connection_dispatch() will never
>dispatch more than one message. One reason is to keep the main loop
>relatively "fair" (avoid e.g. locking up the GUI while we drain a long
>message queue or something). GLib main loop also handles only one X
>event per iteration, etc. Probably the larger reason is just that it
>makes the code easier for me to think about ;-)

Nevermind the "for each" then.

The point is that the message filter is called while the dispatch lock is 
held. Since I can't recurse under those conditions, I can't call to user 
code. On the other hand, the library cannot make the call with an 
unlocked dispatch because the connection could change under its feet. It 
can't know, before calling the binding code, if it will be handled or not 
or if it'll be handled by the message filter or by the object tree 
callback. Or, if I understand the code correctly, by *which* binding 
using that connection.

The solution I am proposing is to give the binding a chance to say "yes, I 
want to handle this message, but I can't right now because the lock is 
held; call me again once you release the lock".

>Anyway, that's why I didn't understand what you meant, I was assuming 1
>message per dispatch.

That's fine too. It removes the need for a local queue. All that the code 
has to do is ref-up the DBusMessage and store it in a local variable. 
Just before returning to the main loop, it calls the binding function 
again.

>However, I think I'm dumping the "IN_PROGRESS" proposal and now
>proposing that we just make the dispatch lock recursive, which I
>hope^H^H^Hthink solves everything. Do you see anything it won't address?

I'm more afraid of the side-effects this could bring. If the lock is 
recursive, the bindings (plural!) are allowed to change the connection 
under the dbus_connection_dispatch's feet. So I'd rather go for the 
solution with the minimal impact, if it solves the same problem.

>The only downside I see is that adding a recursive locks requirement to
>DBusThreadFunctions will require changes to all bindings that provide a
>thread implementation.

If we go that way, then the library could switch to the old behaviour if 
those functions are not available. If they've been working well so far, 
there's no need to change.

-- 
Thiago José Macieira - thiago.macieira AT trolltech.com
Trolltech AS - Sandakerveien 116, NO-0402 Oslo, Norway

pgpCECxSOKtBP.pgp
Description: PGP signature

_______________________________________________
dbus mailing list
dbus-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@xxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dbus