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.

-- 
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