W dniu 14 marca 2012 15:30 użytkownik Patrick Ohly
<[email protected]> napisał:
> Hello Krzesimir!
Hi!
Sorry for double post - forgot to use "Reply to all".
> 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. Such thing happened very often for GIO GDBus
when creating Session - when sync-helper established a DBus connection
to server then server activated Session proxy interface, added Session
to active queue and immediately calls SetActive to mark SessionImpl as
active. It happened quite often that that call went to /dev/null,
because sync-helper didn't yet activated the SessionImpl's DBus
interface. That's why I added a ready signal which child (sync-helper)
can send to parent (server) to note that it is ready (DBus interface
is activated). And this works by sending a custom signal (thus
DBusMessagePtr::create_empty_signal(), some setters and
DBusConnectionPtr::send() are added) to parent and parent installs a
"one time" filter to look for such message (thus some getters in
DBusMessagePtr are added and that filter stuff in DBusConnectionPtr).
By "one time" I mean that the filter is installed as long as no such
signal appeared.
> I stumbled over this when I added an error exception which prevents
> adding new resources to a server which is already shutting down:
>
> void Server::addResource(const Resource_t &resource, const
> boost::function<void()> &callback)
> {
> ==> if (m_shutdownRequested) {
> ==> // don't allow new resources, we cannot activate them
> ==> SE_THROW("server shutting down");
> ==> }
> m_waitingResources.insert(resource);
> checkQueue(callback);
> }
>
> It killed the syncevo-dbus-server because there was nothing catching the
> exception. Regardless whether this is the right approach for solving the
> shutdown problem, throwing and exception should never have that effect.
> Further analysis showed that the call stack is (boost function levels
> omitted):
>
> #0 0x00007ffff092ab40 in __cxa_throw () from
> /usr/lib/x86_64-linux-gnu/libstdc++.so.6
> #1 0x0000000000d710cf in
> SyncEvo::Server::addResource(boost::shared_ptr<SyncEvo::Resource> const&,
> boost::function<void ()> const&) (this=0x7fffffffdb70, resource=...,
> callback=...)
> at /home/pohly/syncevolution/syncevolution/src/dbus/server/server.cpp:447
> #2 0x0000000000d7179e in SyncEvo::Server::startSessionCb
> (this=0x7fffffffdb70, client=..., resource=...,
> result=...) at
> /home/pohly/syncevolution/syncevolution/src/dbus/server/server.cpp:190
> ...
> #8 0x0000000000dc786f in
> SyncEvo::SessionResource::onSessionReady(boost::function<void
> (boost::shared_ptr<SyncEvo::SessionResource> const&)> const&)
> (this=0x7fffe402d430, callback=...)
> at
> /home/pohly/syncevolution/syncevolution/src/dbus/server/session-resource.cpp:403
> ...
> #23 0x0000000000fc5c08 in SyncEvo::ForkExecParent::connectionFilter
> (this=0x7fffe402cf30, conn=..., message=...)
> at /home/pohly/syncevolution/syncevolution/src/syncevo/ForkExec.cpp:219
> ...
> #29 0x0000000000f0389d in GDBusCXX::filter_cb (conn=0x1a7bdb0,
> message=0x1a928f0, user_data=0x7fffe4033a80)
> at
> /home/pohly/syncevolution/syncevolution/src/gdbusxx/gdbus-cxx-bridge.cpp:60
> #30 0x00007ffff177dab8 in on_worker_message_received (worker=<optimized out>,
> message=0x1a928f0,
> user_data=0x1a7bdb0) at
> /tmp/buildd/glib2.0-2.30.2/./gio/gdbusconnection.c:2250
> ...
> #42 0x00007ffff0be27e6 in g_thread_create_proxy (data=0x1a84460) at
> /tmp/buildd/glib2.0-2.30.2/./glib/gthread.c:1962
> #43 0x00007ffff671eb50 in start_thread (arg=<optimized out>) at
> pthread_create.c:304
> #44 0x00007ffff012b90d in clone () at
> ../sysdeps/unix/sysv/linux/x86_64/clone.S:112
> #45 0x0000000000000000 in ?? ()
>
> Two observations:
> 1. GDBusCXX::filter_cb() is a callback for C code, and thus must
> catch all exceptions in the code that it calls, then translate
> them into some kind of C error return code (or whatever the
> right error handling is).
Right, there should be an exception catching clause. Not sure about
right error handling though. Probably an error have to be signaled to
main thread (with g_idle_add?).
> 2. g_dbus_connection_add_filter() says that "filters are run in a
> dedicated message handling thread so they can't block and,
> generally, can't do anything but signal a worker thread". These
> are severe restrictions that aren't documented anywhere in the
> add_filter() method, nor does our code conform to these
> restrictions. For example, it calls the logging system, which is
> not thread-safe.
>
> I think we need a better solution for the problem. FWIW, the main thread
> was idling at the time:
>
> Thread 1 (Thread 0x7ffff7faf960 (LWP 25268)):
> #0 0x00007ffff0120cc3 in *__GI___poll (fds=<optimized out>, nfds=<optimized
> out>, timeout=10)
> at ../sysdeps/unix/sysv/linux/poll.c:87
> #1 0x00007ffff0bbd5d8 in g_main_context_poll (n_fds=4, fds=0x7fffe40329c0,
> timeout=10, context=0x1a75780,
> priority=<optimized out>) at /tmp/buildd/glib2.0-2.30.2/./glib/gmain.c:3391
> #2 g_main_context_iterate (context=0x1a75780, block=<optimized out>,
> dispatch=1, self=<optimized out>)
> at /tmp/buildd/glib2.0-2.30.2/./glib/gmain.c:3071
> #3 0x00007ffff0bbde02 in g_main_loop_run (loop=0x1a75870) at
> /tmp/buildd/glib2.0-2.30.2/./glib/gmain.c:3284
> #4 0x0000000000d75661 in SyncEvo::Server::run (this=0x7fffffffdb70)
> at /home/pohly/syncevolution/syncevolution/src/dbus/server/server.cpp:398
> #5 0x0000000000d6b6c4 in main (argc=1, argv=0x7fffffffe158,
> envp=0x7fffffffe168)
> at /home/pohly/syncevolution/syncevolution/src/dbus/server/main.cpp:141
>
> Perhaps the add_filter callback should be limited to the bare minimum of
> functionality and do nothing more than waking up the main thread, to
> finish whatever work is needed in ForkExec startup?
That is really my bad. I should probably just use g_idle_add, so the
callback will happen in main thread.
>
> --
> 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
