sync aborting: check while starting sync, avoid killing process during testing

The doSync() method does several steps which can take a long time
(for example, send SAN message) before finally entering the while
loop which checks for abort/suspend. In the case that a suspend or
abort request arrives early, doSync() should not complete the
sync setup. Now it checks more often and returns early.

This was found because the fork/exec sync changed the timing so that
the syncevo-dbus-helper ended up being killed by SIGTERM before it
even started syncing. That had the downside that the process didn't
clean up, resulting in many "potentially lost" memory chunks.

Better choose the timing so that doSync() really is ready to handle
the signal. Aborting at that time is also more realistic (normally
sync startup isn't that slow, only valgrind makes it slow).

src/syncevo/SyncContext.cpp

@@ -3260,6 +3260,17 @@ SyncMLStatus SyncContext::doSync()
         signalGuard = SuspendFlags::getSuspendFlags().activate();
     }
 
+    // delay the sync for debugging purposes
+    const char *delay = getenv("SYNCEVOLUTION_SYNC_DELAY");
+    if (delay) {
+        sleep(atoi(delay));
+    }
+
+    if (checkForSuspend() ||
+        checkForAbort()) {
+        return (SyncMLStatus)sysync::LOCERR_USERABORT;
+    }
+
     SyncMLStatus status = STATUS_OK;
     std::string s;
 
@@ -3289,6 +3300,11 @@ SyncMLStatus SyncContext::doSync()
             //by pass the exception if we will try again with legacy SANFormat
         }
 
+        if (checkForSuspend() ||
+            checkForAbort()) {
+            return (SyncMLStatus)sysync::LOCERR_USERABORT;
+        }
+
         if (! status) {
             if (sanFormat.empty()) {
                 SE_LOG_DEBUG (NULL, NULL, "Server Alerted Sync init with SANFormat %d failed, trying with legacy format", version);
@@ -3304,6 +3320,11 @@ SyncMLStatus SyncContext::doSync()
         }
     }
 
+    if (checkForSuspend() ||
+        checkForAbort()) {
+        return (SyncMLStatus)sysync::LOCERR_USERABORT;
+    }
+
     // re-init engine with all sources configured
     string xml, configname;
     initEngine(true);

test/test-dbus.py

@@ -3363,14 +3363,18 @@ END:VCARD''')
         self.assertEqual(self.lastState, "done")
         self.checkSync()
 
-    @property("ENV", "SYNCEVOLUTION_LOCAL_CHILD_DELAY=5")
+    # Killing the syncevo-dbus-helper before it even starts (SYNCEVOLUTION_LOCAL_CHILD_DELAY=5)
+    # is possible, but leads to ugly valgrind warnings about "possibly lost" memory because
+    # SIGTERM really kills the process right away. Better wait until syncing really has started
+    # in the helper (SYNCEVOLUTION_SYNC_DELAY=5). The test is more realistic that way, too.
+    @property("ENV", usingValgrind() and "SYNCEVOLUTION_SYNC_DELAY=55" or "SYNCEVOLUTION_SYNC_DELAY=5")
     @timeout(100)
     def testConcurrency(self):
         """TestLocalSync.testConcurrency - D-Bus server must remain responsive while sync runs"""
         self.setUpConfigs()
         self.setUpListeners(self.sessionpath)
         self.session.Sync("slow", {})
-        time.sleep(2)
+        time.sleep(usingValgrind() and 30 or 3)
         status, error, sources = self.session.GetStatus(utf8_strings=True)
         self.assertEqual(status, "running")
         self.assertEqual(error, 0)