On Thu, 2012-03-15 at 10:21 +0100, Krzesimir Nowak wrote: > W dniu 14 marca 2012 15:30 użytkownik Patrick Ohly > <patrick.ohly(a)intel.com&gt; 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#GDBusConnect... 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.

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

W dniu 15 marca 2012 10:58 użytkownik Krzesimir Nowak <qdlacz(a)gmail.com&gt; napisał: > 2012/3/15 Patrick Ohly <patrick.ohly(a)intel.com&gt;: >> On Thu, 2012-03-15 at 10:21 +0100, Krzesimir Nowak wrote: >>> W dniu 14 marca 2012 15:30 użytkownik Patrick Ohly >>> <patrick.ohly(a)intel.com&gt; 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#GDBusConnect... >> 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-syncevolut... 2. with running said function in idle callback: https://meego.gitorious.org/~krnowak/meego-middleware/krnowaks-syncevolut...

On Thu, 2012-03-15 at 15:43 +0100, Krzesimir Nowak wrote: > W dniu 15 marca 2012 10:58 użytkownik Krzesimir Nowak > <qdlacz(a)gmail.com&gt; napisał: > > 2012/3/15 Patrick Ohly <patrick.ohly(a)intel.com&gt;: > >> On Thu, 2012-03-15 at 10:21 +0100, Krzesimir Nowak wrote: > >>> W dniu 14 marca 2012 15:30 użytkownik Patrick Ohly > >>> <patrick.ohly(a)intel.com&gt; 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#GDBusConnect... > >> 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-syncevolut... > 2. with running said function in idle callback: > https://meego.gitorious.org/~krnowak/meego-middleware/krnowaks-syncevolut... 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)); + }

2012/3/15 Patrick Ohly <patrick.ohly(a)intel.com&gt;: > 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 >> > <qdlacz(a)gmail.com&gt; napisał: >> > > 2012/3/15 Patrick Ohly <patrick.ohly(a)intel.com&gt;: >> > >> On Thu, 2012-03-15 at 10:21 +0100, Krzesimir Nowak wrote: >> > >>> W dniu 14 marca 2012 15:30 użytkownik Patrick Ohly >> > >>> <patrick.ohly(a)intel.com&gt; 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#GDBusConnect... >> > >> 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-syncevolut... >> > 2. with running said function in idle callback: >> > https://meego.gitorious.org/~krnowak/meego-middleware/krnowaks-syncevolut... >> >> 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.

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