From cfdbe08e7559c67ac74461d32c66c09b99bfda51 Mon Sep 17 00:00:00 2001 From: Sam Bosley Date: Thu, 29 Mar 2012 19:04:51 -0700 Subject: [PATCH 1/4] Some ideas for fixing timing issue in astrid.com --- .../astrid/actfm/sync/ActFmSyncService.java | 33 +++++++++++++++++-- .../actfm/sync/ActFmSyncV2Provider.java | 2 ++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/astrid/plugin-src/com/todoroo/astrid/actfm/sync/ActFmSyncService.java b/astrid/plugin-src/com/todoroo/astrid/actfm/sync/ActFmSyncService.java index 34e9c04c8..222285929 100644 --- a/astrid/plugin-src/com/todoroo/astrid/actfm/sync/ActFmSyncService.java +++ b/astrid/plugin-src/com/todoroo/astrid/actfm/sync/ActFmSyncService.java @@ -12,6 +12,8 @@ import java.util.HashSet; import java.util.LinkedList; import java.util.List; import java.util.Queue; +import java.util.concurrent.Semaphore; +import java.util.concurrent.atomic.AtomicInteger; import org.apache.http.entity.mime.MultipartEntity; import org.apache.http.entity.mime.content.ByteArrayBody; @@ -112,8 +114,10 @@ public final class ActFmSyncService { } private final List failedPushes = Collections.synchronizedList(new LinkedList()); - Thread pushRetryThread = null; - Runnable pushRetryRunnable; + private Thread pushRetryThread = null; + private Runnable pushRetryRunnable; + private final AtomicInteger taskPushThreads = new AtomicInteger(0); + private Semaphore waiter = null; public void initialize() { initializeRetryRunnable(); @@ -135,9 +139,17 @@ public final class ActFmSyncService { new Thread(new Runnable() { @Override public void run() { + taskPushThreads.incrementAndGet(); // sleep so metadata associated with task is saved AndroidUtilities.sleepDeep(1000L); pushTaskOnSave(model, setValues); + synchronized(ActFmSyncService.this) { + if (taskPushThreads.decrementAndGet() == 0) { + if (waiter != null) { + waiter.release(); + } + } + } } }).start(); } @@ -235,6 +247,23 @@ public final class ActFmSyncService { } } + public void waitUntilEmpty() { + synchronized(this) { + if (taskPushThreads.get() > 0) + waiter = new Semaphore(0); + else + return; + } + try { + waiter.acquire(); + } catch (InterruptedException e) { + // Ignored + } + synchronized(this) { + waiter = null; + } + } + // --- data push methods /** diff --git a/astrid/plugin-src/com/todoroo/astrid/actfm/sync/ActFmSyncV2Provider.java b/astrid/plugin-src/com/todoroo/astrid/actfm/sync/ActFmSyncV2Provider.java index d3b4f085d..4633c1c39 100644 --- a/astrid/plugin-src/com/todoroo/astrid/actfm/sync/ActFmSyncV2Provider.java +++ b/astrid/plugin-src/com/todoroo/astrid/actfm/sync/ActFmSyncV2Provider.java @@ -118,6 +118,7 @@ public class ActFmSyncV2Provider extends SyncV2Provider { startTagFetcher(callback, finisher); + actFmSyncService.waitUntilEmpty(); startTaskFetcher(manual, callback, finisher); callback.incrementProgress(50); @@ -255,6 +256,7 @@ public class ActFmSyncV2Provider extends SyncV2Provider { fetchTagData(tagData, noRemoteId, manual, callback, finisher); if(!noRemoteId) { + actFmSyncService.waitUntilEmpty(); fetchTasksForTag(tagData, manual, callback, finisher); fetchUpdatesForTag(tagData, manual, callback, finisher); } From cb95621ddc08a287e203253f545cb7ee5a6a9f41 Mon Sep 17 00:00:00 2001 From: Sam Bosley Date: Fri, 30 Mar 2012 11:22:31 -0700 Subject: [PATCH 2/4] Use a condition variable for the actfm sync timing --- .../astrid/actfm/sync/ActFmSyncService.java | 28 +++++-------------- 1 file changed, 7 insertions(+), 21 deletions(-) diff --git a/astrid/plugin-src/com/todoroo/astrid/actfm/sync/ActFmSyncService.java b/astrid/plugin-src/com/todoroo/astrid/actfm/sync/ActFmSyncService.java index 222285929..9f6057068 100644 --- a/astrid/plugin-src/com/todoroo/astrid/actfm/sync/ActFmSyncService.java +++ b/astrid/plugin-src/com/todoroo/astrid/actfm/sync/ActFmSyncService.java @@ -12,7 +12,6 @@ import java.util.HashSet; import java.util.LinkedList; import java.util.List; import java.util.Queue; -import java.util.concurrent.Semaphore; import java.util.concurrent.atomic.AtomicInteger; import org.apache.http.entity.mime.MultipartEntity; @@ -24,6 +23,7 @@ import org.json.JSONObject; import android.content.ContentValues; import android.content.Context; import android.graphics.Bitmap; +import android.os.ConditionVariable; import android.os.Handler; import android.os.Looper; import android.text.TextUtils; @@ -117,7 +117,7 @@ public final class ActFmSyncService { private Thread pushRetryThread = null; private Runnable pushRetryRunnable; private final AtomicInteger taskPushThreads = new AtomicInteger(0); - private Semaphore waiter = null; + private final ConditionVariable waitUntilEmpty = new ConditionVariable(); public void initialize() { initializeRetryRunnable(); @@ -140,15 +140,12 @@ public final class ActFmSyncService { @Override public void run() { taskPushThreads.incrementAndGet(); + waitUntilEmpty.close(); // sleep so metadata associated with task is saved AndroidUtilities.sleepDeep(1000L); pushTaskOnSave(model, setValues); - synchronized(ActFmSyncService.this) { - if (taskPushThreads.decrementAndGet() == 0) { - if (waiter != null) { - waiter.release(); - } - } + if (taskPushThreads.decrementAndGet() == 0) { + waitUntilEmpty.open(); } } }).start(); @@ -248,19 +245,8 @@ public final class ActFmSyncService { } public void waitUntilEmpty() { - synchronized(this) { - if (taskPushThreads.get() > 0) - waiter = new Semaphore(0); - else - return; - } - try { - waiter.acquire(); - } catch (InterruptedException e) { - // Ignored - } - synchronized(this) { - waiter = null; + while (taskPushThreads.get() > 0) { + waitUntilEmpty.block(); } } From 02d6d30d834a2d81e7cf56953791adfca1da7386 Mon Sep 17 00:00:00 2001 From: Sam Bosley Date: Fri, 30 Mar 2012 11:35:54 -0700 Subject: [PATCH 3/4] Threads launching threads --- .../astrid/actfm/sync/ActFmSyncService.java | 11 ++-- .../actfm/sync/ActFmSyncV2Provider.java | 50 +++++++++++-------- 2 files changed, 36 insertions(+), 25 deletions(-) diff --git a/astrid/plugin-src/com/todoroo/astrid/actfm/sync/ActFmSyncService.java b/astrid/plugin-src/com/todoroo/astrid/actfm/sync/ActFmSyncService.java index 9f6057068..890a9ccb7 100644 --- a/astrid/plugin-src/com/todoroo/astrid/actfm/sync/ActFmSyncService.java +++ b/astrid/plugin-src/com/todoroo/astrid/actfm/sync/ActFmSyncService.java @@ -142,10 +142,13 @@ public final class ActFmSyncService { taskPushThreads.incrementAndGet(); waitUntilEmpty.close(); // sleep so metadata associated with task is saved - AndroidUtilities.sleepDeep(1000L); - pushTaskOnSave(model, setValues); - if (taskPushThreads.decrementAndGet() == 0) { - waitUntilEmpty.open(); + try { + AndroidUtilities.sleepDeep(1000L); + pushTaskOnSave(model, setValues); + } finally { + if (taskPushThreads.decrementAndGet() == 0) { + waitUntilEmpty.open(); + } } } }).start(); diff --git a/astrid/plugin-src/com/todoroo/astrid/actfm/sync/ActFmSyncV2Provider.java b/astrid/plugin-src/com/todoroo/astrid/actfm/sync/ActFmSyncV2Provider.java index 4633c1c39..dc7cac425 100644 --- a/astrid/plugin-src/com/todoroo/astrid/actfm/sync/ActFmSyncV2Provider.java +++ b/astrid/plugin-src/com/todoroo/astrid/actfm/sync/ActFmSyncV2Provider.java @@ -106,22 +106,26 @@ public class ActFmSyncV2Provider extends SyncV2Provider { // --- synchronize active tasks @Override - public void synchronizeActiveTasks(boolean manual, + public void synchronizeActiveTasks(final boolean manual, final SyncResultCallback callback) { - callback.started(); - callback.incrementMax(100); + new Thread(new Runnable() { + public void run() { + callback.started(); + callback.incrementMax(100); - final AtomicInteger finisher = new AtomicInteger(2); + final AtomicInteger finisher = new AtomicInteger(2); - actFmPreferenceService.recordSyncStart(); + actFmPreferenceService.recordSyncStart(); - startTagFetcher(callback, finisher); + startTagFetcher(callback, finisher); - actFmSyncService.waitUntilEmpty(); - startTaskFetcher(manual, callback, finisher); + actFmSyncService.waitUntilEmpty(); + startTaskFetcher(manual, callback, finisher); - callback.incrementProgress(50); + callback.incrementProgress(50); + } + }).start(); } /** fetch changes to tags */ @@ -239,29 +243,33 @@ public class ActFmSyncV2Provider extends SyncV2Provider { // --- synchronize list @Override - public void synchronizeList(Object list, boolean manual, + public void synchronizeList(Object list, final boolean manual, final SyncResultCallback callback) { if(!(list instanceof TagData)) return; - TagData tagData = (TagData) list; + final TagData tagData = (TagData) list; final boolean noRemoteId = tagData.getValue(TagData.REMOTE_ID) == 0; - callback.started(); - callback.incrementMax(100); + new Thread(new Runnable() { + public void run() { + callback.started(); + callback.incrementMax(100); - final AtomicInteger finisher = new AtomicInteger(3); + final AtomicInteger finisher = new AtomicInteger(3); - fetchTagData(tagData, noRemoteId, manual, callback, finisher); + fetchTagData(tagData, noRemoteId, manual, callback, finisher); - if(!noRemoteId) { - actFmSyncService.waitUntilEmpty(); - fetchTasksForTag(tagData, manual, callback, finisher); - fetchUpdatesForTag(tagData, manual, callback, finisher); - } + if(!noRemoteId) { + actFmSyncService.waitUntilEmpty(); + fetchTasksForTag(tagData, manual, callback, finisher); + fetchUpdatesForTag(tagData, manual, callback, finisher); + } - callback.incrementProgress(50); + callback.incrementProgress(50); + } + }).start(); } private void fetchTagData(final TagData tagData, final boolean noRemoteId, From 7e1335adbc4965e3f2b79eff2f947069876aa6c1 Mon Sep 17 00:00:00 2001 From: Sam Bosley Date: Fri, 30 Mar 2012 11:50:31 -0700 Subject: [PATCH 4/4] Fixed a synchronization bug --- .../com/todoroo/astrid/actfm/sync/ActFmSyncService.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/astrid/plugin-src/com/todoroo/astrid/actfm/sync/ActFmSyncService.java b/astrid/plugin-src/com/todoroo/astrid/actfm/sync/ActFmSyncService.java index 890a9ccb7..2d8bdbd41 100644 --- a/astrid/plugin-src/com/todoroo/astrid/actfm/sync/ActFmSyncService.java +++ b/astrid/plugin-src/com/todoroo/astrid/actfm/sync/ActFmSyncService.java @@ -139,8 +139,8 @@ public final class ActFmSyncService { new Thread(new Runnable() { @Override public void run() { - taskPushThreads.incrementAndGet(); waitUntilEmpty.close(); + taskPushThreads.incrementAndGet(); // sleep so metadata associated with task is saved try { AndroidUtilities.sleepDeep(1000L); @@ -248,9 +248,7 @@ public final class ActFmSyncService { } public void waitUntilEmpty() { - while (taskPushThreads.get() > 0) { - waitUntilEmpty.block(); - } + waitUntilEmpty.block(); } // --- data push methods