Re: [SyncEvolution] server class + fork/exec dbus-server

On Fri, 2012-03-16 at 15:35 +0100, Patrick Ohly wrote: I've finished the initial server rewrite, see ccs-pohly. I have also done the auto sync manager rewrite that I had planned. I think the key difference (only run a subset of Session in the helper, no dead code for running multiple sessions in parallel) makes the code easier and avoided changes elsewhere in the server (like an entirely different internal Connection API, asynchronous status queries, etc.). The new design is less ambitious in some ways. For example, backends are still loaded and activated in the main syncevo-dbus-server, and there is no support for running multiple sync sessions in parallel. But I believe that this can be added later without design changes. All tests passed on my desktop, now I am testing on the nightly test machine. However, while working on this I noticed quite a few areas for which there are no tests. For example, running a command line operation is not tested. Support for it is implemented, but only without the output handling. Before adding that, the tests from the Cmdline unit tests should be incorporated into test-dbus.py. That way it can be verified that the tests work (because they fail initially with the new branch) and developing the feature becomes easier (because testing doesn't have to be manual). The main commit message has the details (pri1 is something that I'll work on immediately, pri2 is necessary before 1.3, pri3 is optional). Help for anything in pri2 or pri3 is welcome. commit 2250ea0ac8adb060eaacd73d7562643509cdb06e Author: Patrick Ohly <patrick.ohly(a)intel.com&gt; Date: Mon Mar 26 17:19:25 2012 +0200 D-Bus server: run operations in syncevo-dbus-helper This commit moves running the blocking syncing, database restore and command line execution into the syncevo-dbus-helper process. The advantage is that the main syncevo-dbus-server remains responsive under all circumstances (fully asynchronous now) and suffers less from memory leaks and/or crashes during a sync. All test-dbus.py tests pass. The core idea behind the new architecture is that Session remains the D-Bus facing side of a session. It continues to run inside syncevo-dbus-server and uses the syncevo-dbus-helper transparently via a custom D-Bus interface between the two processes. State changes of the helper are mirrored in the server. Later the helper might also be used multiple times in a Session. For example, anything related to loading backends should be moved into the helper (currently the "is config usable" check still runs in the syncevo-dbus-server and needs to load/initialize backends). The startup code of the helper already handles that (see boolean result of operation callback), but it is not used yet in practice. At the moment, only the helper provides a D-Bus API. It sends out signals when it needs information from the server. The server watches those and replies when ready. It has the disadvantage that the helper is not automatically notified with a "connection lost" error when waiting for information from the server. This is not currently handled (see below). Might revise this approach as part of resolving the issue. The problem of "helper died unexpectedly" is already handled, by not returning a D-Bus method reply until the requested operation is completed. The Connection class continues to use such a Session, as before. It's now fully asynchronous and exchanges messages with the helper via the Session class. Inside syncevo-dbus-server, boost::signals2 and the dbus-callbacks infrastructure for asynchronous methods execution are used heavily now. The glib event loop is entered exactly once and only left to shut down. The AutoSyncManager was completely rewritten. The data structure is a lot simpler now (basically just a cache of transient information about a sync config and the relevant config properties that define auto syncing). The main work happens inside the schedule() call, which verifies whether a session can run and, if not possible for some reasons, ensures that it gets invoked again when that blocker is gone (timeout over, server idle, etc.). The new code also uses signals/slots instead of explicit coupling between the different classes. pri3: syncevo-dbus-helper and syncevo-local-sync are conceptually very similar. Should investigate whether a single executable can serve both functions. pri1: All code still lives inside the src/dbus/server directory. This simplifies checking differences in partly modified files like dbus-sync.cpp. The next commit will move the helper files. pri1: Redirecting output from a running sync or the command line operation in the helper to the client of syncevo-dbus-server is not implemented. Need to write tests for the old implementation first, then implement the missing functionality. pri2: A sync is meant to finish even if the syncevo-dbus-server gets killed. On the other hand, in some cases (pending password request, waiting for next message from Connection) the helper needs to detect that syncevo-dbus-server process died, because the helper will never get a response. Need a test for these aspects. pri2: When syncevo-dbus-server is invoked without SYNCEVOLUTION_DEBUG, it logs to the syslog instead of stdout. Need a test. The syncevo-dbus-server now sends the final "Session.StatusChanged done" signal immediately. The old implementation accidentally delayed sending that for 100 seconds. The revised test-dbus.py checks for more "session done" quit events to cover this fix. -- 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.