7:54 a.m.
http://bugzilla.moblin.org/show_bug.cgi?id=7555
--- Comment #11 from pohly <patrick.ohly(a)intel.com> 2009-11-09 12:54:20 PST ---
Yongsheng, I cherry-picked your patch (so that you can "git rebase" without
running into merge conflicts, and because I wanted to reuse some parts). Then I
rewrote large parts of the signal handling, once I started to think about the
problem and test it in various situations as part of the server session
handling.
Please review the "signal-handling" branch with the following commit:
syncevo-dbus-server + syncevolution: fixed signal handling and D-Bus
suspend/abort/shutdown (MB#7555)
This patch fixes signal handling and shutdown of syncevo-dbus-server
when signals were received. This problems were found in combination
with automated tests via test-dbus.py.
Signals handled by the signal handlers in the core had no effect on
syncevo-dbus-server, which called its own event loop once the
suspended or aborted sync returned. Signals received while outside
of a sync and outside of an event loop also had no effect.
Now signals are always caught by the niam() function in
syncevo-dbus-server. It uses the new SyncContext::handleSignal() API
to tell the core engine about it and in addition, remembers that
syncevo-dbus-server is meant to shut down (suspendRequested). This is
then checked by the syncevo-dbus-server event loop (DBusServer::run())
before blocking again.
The implementation of Session.Abort() and Session.Suspend() didn't
work. isSuspend() and isAbort() mixed up the state. They also did not
wake up DBusTransportAgent, so the syncevo-dbus-server might get stuck
although an abort was already requested. Partially fixed by adding
g_main_loop_quit(). It's not entirely sure whether g_main_loop_quit()
is reentrant (see below). There is other code which uses it in signal
handlers, but that doesn't mean that this is correct. The right long
term solution would be:
- avoid entering and leaving the event loop
- feed signals into the event loop as normal events
SoupTransportAgent solves this by polling for a signal once per second.
This also should be improved.
Because such code was used in several places for a connection, that common
code was moved into Connection::wakeupSession().
Things which went in unnoticed when the original signal handling was
merged:
- SyncContext::getSuspendFlags() should not allow the caller to modify
the SyncContext state. Made the returned SuspendFlags const.
- A signal handler may only call re-entrant functions (Stevens, "Advanced
Programming in a Unix Environment", 10.6).
All printing therefore has to be done outside of the signal handlers.
Now the signal handlers just set a message and the regular code
checking for abort/suspend prints it. There's a slight race condition
(a message might get overwritten before it is printed), but printing
just the last message might actually make more sense. There might be
a slight delay between receiving the signal and printing now.
--
Configure bugmail: http://bugzilla.moblin.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching someone on the CC list of the bug.