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