W dniu 15 marca 2012 17:54 użytkownik Krzesimir Nowak
<[email protected]> napisał:
> 2012/3/15 Patrick Ohly <[email protected]>:
>> On Thu, 2012-03-15 at 16:03 +0100, Patrick Ohly wrote:
>>> On Thu, 2012-03-15 at 15:43 +0100, Krzesimir Nowak wrote:
>>> > W dniu 15 marca 2012 10:58 użytkownik Krzesimir Nowak
>>> > <[email protected]> napisał:
>>> > > 2012/3/15 Patrick Ohly <[email protected]>:
>>> > >> On Thu, 2012-03-15 at 10:21 +0100, Krzesimir Nowak wrote:
>>> > >>> W dniu 14 marca 2012 15:30 użytkownik Patrick Ohly
>>> > >>> <[email protected]> napisał:
>>> > >>> > Hello Krzesimir!
>>> > >>> >
>>> > >>> > You added the add/remove_filter() methods to DBusConnectionPtr, 
>>> > >>> > with the
>>> > >>> > comment "those additions will be needed for ForkExec ready message
>>> > >>> > handling" in the commit message.
>>> > >>> >
>>> > >>> > The methods themselves are not documented. Can you explain a bit how
>>> > >>> > this is meant to work?
>>> > >>>
>>> > >>> The history is that I needed to add a new signal to ForkExec, because
>>> > >>> activation of DBus interface on child side was racing with using this
>>> > >>> interface on parent side.
>>> > >>
>>> > >> Wouldn't it be easier to delay message processing on the child side
>>> > >> until the child is set up, then enable the message processing?
>>> > >>
>>> > >> http://developer.gnome.org/gio/unstable/GDBusConnection.html#GDBusConnectionFlags
>>> > >> mentions G_DBUS_CONNECTION_FLAGS_DELAY_MESSAGE_PROCESSING and
>>> > >> g_dbus_connection_start_message_processing() for this purpose.
>>> > >>
>>> > >> Then the parent can start making method calls right away. They simply
>>> > >> will not be processed before the child is really ready to handle them.
>>> > >
>>> > > I have not noticed that before - I will check it.
>>> >
>>> > Nope, still racy. I tested that by running a loop with 100 iterations
>>> > of TestDBusServerPresence and breaking it after first failure. It got
>>> > me as far as 34 iterations. Usually first 15-20 iteration succeeded.
>>> > Failures on 1 iteration weren't all that rare. I have created two
>>> > quick-and-dirty branches with my two attempts to solve the problem
>>> > with g_dbus_connection_start_message_processing():
>>> > 1. with running said function directly:
>>> > https://meego.gitorious.org/~krnowak/meego-middleware/krnowaks-syncevolution/commits/css-with-direct-msg-proc
>>> > 2. with running said function in idle callback:
>>> > https://meego.gitorious.org/~krnowak/meego-middleware/krnowaks-syncevolution/commits/css-with-idle-msg-proc
>>>
>>> Is the logic of the if() check accidentally reverted here?
>>>
>>> void ForkExecChild::connect(bool delayed /* = false */)
>>>
>>> +    if (delayed) {
>>> +        m_conn = 
>>> GDBusCXX::DBusConnectionPtr(dbus_get_bus_connection(address,
>>> +                                                                     
>>> &dbusError));
>>> +    } else {
>>> +        m_conn = 
>>> GDBusCXX::DBusConnectionPtr(dbus_get_bus_connection_delayed(address,
>>> +                                                                           
>>>   &dbusError));
>>> +    }
>>
>> Inverting that logic so that message processing really gets delayed as
>> intended leads makes your test succeed. It passed all 100 rounds without
>> problems, whereas it failed quickly with the unchanged if().
>>
>> So it seems to me that this is the right way to go. Regarding the code,
>> can we introduce flags for dbus_get_bus_connection() instead of creating
>> multiple copies of it? That'll scale better in case that we find a need
>> for other variations.
>
> That is almost exactly what I did - but instead of adding flags I just
> added a bool parameter. Right now I am creating some commits. I will
> let you know when I push this.

Pushed here (forced update):
https://meego.gitorious.org/~krnowak/meego-middleware/krnowaks-syncevolution/commits/css-with-direct-msg-proc

>> --
>> Best Regards, Patrick Ohly
>>
>> The content of this message is my personal opinion only and although
>> I am an employee of Intel, the statements I make here in no way
>> represent Intel's position on the issue, nor am I authorized to speak
>> on behalf of Intel on this matter.
>>
>>
_______________________________________________
SyncEvolution mailing list
[email protected]
http://lists.syncevolution.org/listinfo/syncevolution

Reply via email to