On Fr, 2012-01-20 at 10:53 +0100, Chris Kühl wrote:
On Thu, Jan 19, 2012 at 4:19 PM, Patrick Ohly
> On Mi, 2012-01-18 at 16:55 +0100, Chris Kühl wrote:
>> I've renamed my branch to concurrent-sync-sessions and rebased onto
>> master. I'm now going through and making required changes to get tests
>> to work.
> Note that I pushed another change onto a new "signal-handling" branch.
> This affects you because syncevo-dbus-server will have to relay
> suspend/abort requests. The helper process will have to deal with
> signals similar to syncevo-local-sync helper in that branch.
> Do these changes make sense?
Yes, this looks fine.
They are on their way towards master. Running one more test now.
After looking into what it would take to have all backends react to
abort requests in a timely manner I have come to the conclusion that it
will be much simpler to just let the process running them die when
receiving a signal. Teaching libs with a synchronous API, like libneon
and the activesyncd client libraries, how to watch additional events
will be hard.
For syncevo-local-sync I have implemented that and intend to merge it
soon, see for-master/signal-handling branch.
Another patch modifies the syncevo-dbus-server. It's a step towards
gettting rid of SyncContext::checkForAbort/Suspend(). This is not
absolutely necessary at the moment and probably conflicts with your
server rewrite, so I intend to keep it on a branch for now.
Comments welcome, though:
Author: Patrick Ohly <patrick.ohly(a)intel.com> 2012-01-20 18:20:29
Committer: Patrick Ohly <patrick.ohly(a)intel.com> 2012-01-20 20:43:31
Parent: 1fa8a5d94e8352f6ccd7574d275bc88102a09ed3 (local sync: better error reporting when
child finds problem)
Branches: remotes/origin/signal-handling, signal-handling
D-Bus server: set global suspend/abort state
The D-Bus server implemented suspend/abort of a session only in its
checkForSuspend/Abort() implementation my looking at the sessions
current state. The downside is that polling is necessary to detect
that state change.
This commit introduces the possibility to temporarily change the
global SuspendFlags state to abort resp. suspend. The duration of that
change is determined by the caller, which needs to keep a shared
pointer alive as long as it wants to remain in that
state. SuspendFlags itself doesn't keep the shared pointer; it merely
checks via weak pointers whether the shared ones still exist.
This approach ensures that if the caller gets destructed, the global
state automatically returns what it was before. This is necessary in
the D-Bus server because it runs one session after the other and
aborting one session should never affect another.
However, sessions stay around longer than they are active (= own the
global state). Therefore it is necessary to never modify the global
state before the session is active and restore it as soon as the
session becomes inactive again. The redundancy between session state
and global state is encapsulated in the new SyncStateOwner class which
replaces a mere enum. Now the state assignment triggers additional
code which mirrors the session's state in SuspendFlags.
It can also do additional sanity checks, like preventing a transition
from "abort" to "running". Previously, the abort or suspend
a not yet running session most likely was ignored by overwriting it
with SYNC_RUNNING in run().
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.