Improvements for concurrency

Marked some Post members for when they may be modified.
Closed, archived, sticky etc. is now only modified on the main thread.

BoardManager.getBoardByValue still gets called from multiple threads, next thing to improve.

Clover/app/src/main/java/org/floens/chan/chan/ChanLoader.java

@@ -19,6 +19,7 @@ package org.floens.chan.chan;
 
 import android.text.TextUtils;
 
+import com.android.volley.RequestQueue;
 import com.android.volley.Response;
 import com.android.volley.VolleyError;
 
@@ -39,7 +40,7 @@ import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
 
-public class ChanLoader {
+public class ChanLoader implements Response.ErrorListener, Response.Listener<ChanReaderRequest.ChanReaderResponse> {
     private static final String TAG = "ChanLoader";
     private static final ScheduledExecutorService executor = Executors.newSingleThreadScheduledExecutor();
 
@@ -47,6 +48,7 @@ public class ChanLoader {
 
     private final List<ChanLoaderCallback> listeners = new ArrayList<>();
     private final Loadable loadable;
+    private final RequestQueue volleyRequestQueue;
     private ChanThread thread;
 
     private boolean destroyed = false;
@@ -64,6 +66,8 @@ public class ChanLoader {
         if (loadable.mode == Loadable.Mode.BOARD) {
             loadable.mode = Loadable.Mode.CATALOG;
         }
+
+        volleyRequestQueue = Chan.getVolleyRequestQueue();
     }
 
     /**
@@ -149,11 +153,7 @@ public class ChanLoader {
     public void requestMoreData() {
         clearTimer();
 
-        if (loadable.isThreadMode()) {
+        if (loadable.isThreadMode() && request == null) {
-            if (request != null) {
-                return;
-            }
-
             request = getData();
         }
     }
@@ -166,9 +166,6 @@ public class ChanLoader {
         requestMoreData();
     }
 
-    /**
-     * @return Returns if this loader is currently loading
-     */
     public boolean isLoading() {
         return request != null;
     }
@@ -193,10 +190,102 @@ public class ChanLoader {
         return thread;
     }
 
+    @Override
+    public void onResponse(ChanReaderRequest.ChanReaderResponse response) {
+        request = null;
+        if (destroyed)
+            return;
+
+        if (response.posts.size() == 0) {
+            onErrorResponse(new VolleyError("Post size is 0"));
+            return;
+        }
+
+        if (thread == null) {
+            thread = new ChanThread(loadable, new ArrayList<Post>());
+        }
+
+        thread.posts.clear();
+        thread.posts.addAll(response.posts);
+
+        processResponse(response);
+
+        if (TextUtils.isEmpty(loadable.title)) {
+            loadable.title = PostHelper.getTitle(thread.op, loadable);
+        }
+
+        for (Post post : thread.posts) {
+            post.title = loadable.title;
+        }
+
+        lastLoadTime = Time.get();
+
+        if (loadable.isThreadMode()) {
+            setTimer(response.posts.size());
+        }
+
+        for (ChanLoaderCallback l : listeners) {
+            l.onChanLoaderData(thread);
+        }
+    }
+
+    @Override
+    public void onErrorResponse(VolleyError error) {
+        request = null;
+        if (destroyed)
+            return;
+
+        Logger.i(TAG, "Loading error", error);
+
+        clearTimer();
+
+        for (ChanLoaderCallback l : listeners) {
+            l.onChanLoaderError(error);
+        }
+    }
+
+    /**
+     * Final processing af a response that needs to happen on the main thread.
+     *
+     * @param response Response to process
+     */
+    private void processResponse(ChanReaderRequest.ChanReaderResponse response) {
+        if (loadable.isThreadMode() && thread.posts.size() > 0) {
+            // Replace some op parameters to the real op (index 0).
+            // This is done on the main thread to avoid race conditions.
+            Post realOp = thread.posts.get(0);
+            thread.op = realOp;
+            Post fakeOp = response.op;
+            if (fakeOp != null) {
+                thread.closed = realOp.closed = fakeOp.closed;
+                thread.archived = realOp.archived = fakeOp.archived;
+                realOp.sticky = fakeOp.sticky;
+                realOp.replies = fakeOp.replies;
+                realOp.images = fakeOp.images;
+                realOp.uniqueIps = fakeOp.uniqueIps;
+            } else {
+                Logger.e(TAG, "Thread has no op!");
+            }
+        }
+
+        for (Post sourcePost : thread.posts) {
+            sourcePost.repliesFrom.clear();
+
+            for (Post replyToSource : thread.posts) {
+                if (replyToSource != sourcePost) {
+                    if (replyToSource.repliesTo.contains(sourcePost.no)) {
+                        sourcePost.repliesFrom.add(replyToSource.no);
+                    }
+                }
+            }
+        }
+    }
+
     private void setTimer(int postCount) {
         clearTimer();
 
         if (postCount > lastPostCount) {
+            lastPostCount = postCount;
             currentTimeout = 0;
         } else {
             currentTimeout++;
@@ -209,8 +298,6 @@ public class ChanLoader {
             currentTimeout = 4; // At least 60 seconds in the background
         }
 
-        lastPostCount = postCount;
-
         if (autoReload) {
             Runnable pendingRunnable = new Runnable() {
                 @Override
@@ -243,76 +330,13 @@ public class ChanLoader {
         Logger.d(TAG, "Requested " + loadable.board + ", " + loadable.no);
 
         List<Post> cached = thread == null ? new ArrayList<Post>() : thread.posts;
-        ChanReaderRequest request = ChanReaderRequest.newInstance(loadable, cached,
+        ChanReaderRequest request = ChanReaderRequest.newInstance(loadable, cached, this, this);
-                new Response.Listener<List<Post>>() {
-                    @Override
-                    public void onResponse(List<Post> list) {
-                        ChanLoader.this.request = null;
-                        onData(list);
-                    }
-                }, new Response.ErrorListener() {
-                    @Override
-                    public void onErrorResponse(VolleyError error) {
-                        ChanLoader.this.request = null;
-                        onError(error);
-                    }
-                }
-        );
 
-        Chan.getVolleyRequestQueue().add(request);
+        volleyRequestQueue.add(request);
 
         return request;
     }
 
-    private void onData(List<Post> result) {
-        if (destroyed)
-            return;
-
-        if (thread == null) {
-            thread = new ChanThread(loadable, new ArrayList<Post>());
-        }
-
-        thread.posts.clear();
-        thread.posts.addAll(result);
-
-        if (loadable.isThreadMode() && thread.posts.size() > 0) {
-            thread.op = thread.posts.get(0);
-            thread.closed = thread.op.closed;
-            thread.archived = thread.op.archived;
-        }
-
-        if (TextUtils.isEmpty(loadable.title)) {
-            loadable.title = PostHelper.getTitle(thread.op, loadable);
-        }
-
-        for (Post post : thread.posts) {
-            post.title = loadable.title;
-        }
-
-        lastLoadTime = Time.get();
-
-        if (loadable.isThreadMode()) {
-            setTimer(result.size());
-        }
-
-        for (ChanLoaderCallback l : listeners) {
-            l.onChanLoaderData(thread);
-        }
-    }
-
-    private void onError(VolleyError error) {
-        if (destroyed)
-            return;
-
-        Logger.e(TAG, "Loading error");
-
-        clearTimer();
-
-        for (ChanLoaderCallback l : listeners) {
-            l.onChanLoaderError(error);
-        }
-    }
-
     public interface ChanLoaderCallback {
         void onChanLoaderData(ChanThread result);
 

Clover/app/src/main/java/org/floens/chan/chan/ChanParser.java

@@ -33,6 +33,7 @@ import org.floens.chan.Chan;
 import org.floens.chan.core.model.Post;
 import org.floens.chan.core.model.PostLinkable;
 import org.floens.chan.core.settings.ChanSettings;
+import org.floens.chan.database.DatabaseManager;
 import org.floens.chan.ui.theme.Theme;
 import org.floens.chan.ui.theme.ThemeHelper;
 import org.floens.chan.utils.Logger;
@@ -60,6 +61,11 @@ public class ChanParser {
     private static final Pattern colorPattern = Pattern.compile("color:#([0-9a-fA-F]*)");
 
     private static ChanParser instance = new ChanParser();
+    private final DatabaseManager databaseManager;
+
+    public ChanParser() {
+        databaseManager = Chan.getDatabaseManager();
+    }
 
     public static ChanParser getInstance() {
         return instance;
@@ -404,8 +410,7 @@ public class ChanParser {
                     }
 
                     // Append You when it's a reply to an saved reply
-                    // todo synchronized
+                    if (databaseManager.isSavedReply(post.board, id)) {
-                    if (Chan.getDatabaseManager().isSavedReply(post.board, id)) {
                         key += " (You)";
                     }
                 }

Clover/app/src/main/java/org/floens/chan/core/manager/BoardManager.java

@@ -52,6 +52,7 @@ public class BoardManager {
         loadFromServer();
     }
 
+    // TODO: synchronize
     public Board getBoardByValue(String value) {
         return allBoardsByValue.get(value);
     }

Clover/app/src/main/java/org/floens/chan/core/model/Post.java

@@ -21,20 +21,25 @@ import android.text.SpannableString;
 import android.text.TextUtils;
 
 import org.floens.chan.Chan;
-import org.floens.chan.chan.ChanUrls;
 import org.floens.chan.chan.ChanParser;
+import org.floens.chan.chan.ChanUrls;
 import org.jsoup.parser.Parser;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Random;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 /**
- * Contains all data needed to represent a single post.
+ * Contains all data needed to represent a single post.<br>
+ * Call {@link #finish()} to parse the comment etc. The post data is invalid if finish returns false.<br>
+ * This class has members that are threadsafe and some that are not, see the source for more info.
  */
 public class Post {
     private static final Random random = new Random();
 
+    // *** These next members don't get changed after finish() is called. Effectively final. ***
     public String board;
     public int no = -1;
     public int resto = -1;
@@ -46,42 +51,25 @@ public class Post {
     public long tim = -1;
     public String ext;
     public String filename;
-    public int replies = -1;
     public int imageWidth;
     public int imageHeight;
     public boolean hasImage = false;
     public String thumbnailUrl;
     public String imageUrl;
-    public boolean sticky = false;
-    public boolean closed = false;
-    public boolean archived = false;
     public String tripcode = "";
     public String id = "";
     public String capcode = "";
     public String country = "";
     public String countryName = "";
     public long time = -1;
-    public boolean isSavedReply = false;
-    public String title = "";
     public int fileSize;
-    public int images = -1;
     public String rawComment;
     public String countryUrl;
     public boolean spoiler = false;
-    public int uniqueIps = 1;
-
-    public boolean deleted = false;
-
     /**
-     * This post replies to the these ids
+     * This post replies to the these ids. Is an unmodifiable list after finish().
      */
     public List<Integer> repliesTo = new ArrayList<>();
-
-    /**
-     * These ids replied to this post
-     */
-    public List<Integer> repliesFrom = new ArrayList<>();
-
     public final ArrayList<PostLinkable> linkables = new ArrayList<>();
     public boolean parsedSpans = false;
     public SpannableString subjectSpan;
@@ -91,11 +79,25 @@ public class Post {
     public SpannableString capcodeSpan;
     public CharSequence nameTripcodeIdCapcodeSpan;
 
-    public Post() {
+    // *** These next members may only change on the main thread after finish(). ***
-    }
+    public boolean sticky = false;
+    public boolean closed = false;
+    public boolean archived = false;
+    public int replies = -1;
+    public int images = -1;
+    public int uniqueIps = 1;
+    public String title = "";
+    /**
+     * These ids replied to this post. Only modify this on the main thread.
+     */
+    public List<Integer> repliesFrom = new ArrayList<>();
+
+    // *** Threadsafe members, may be read and modified on any thread. ***
+    public AtomicBoolean deleted = new AtomicBoolean(false);
+    public AtomicBoolean isSavedReply = new AtomicBoolean(false);
 
     /**
-     * Finish up the data
+     * Finish up the data: parse the comment, check if the data is valid etc.
      *
      * @return false if this data is invalid
      */
@@ -134,6 +136,8 @@ public class Post {
 
         ChanParser.getInstance().parse(this);
 
+        repliesTo = Collections.unmodifiableList(repliesTo);
+
         return true;
     }
 }

Clover/app/src/main/java/org/floens/chan/core/net/ChanReaderRequest.java

@@ -26,22 +26,23 @@ import org.floens.chan.Chan;
 import org.floens.chan.chan.ChanUrls;
 import org.floens.chan.core.model.Loadable;
 import org.floens.chan.core.model.Post;
+import org.floens.chan.utils.Logger;
 
-import java.io.IOException;
 import java.util.ArrayList;
-import java.util.HashSet;
 import java.util.List;
-import java.util.Set;
 
-public class ChanReaderRequest extends JsonReaderRequest<List<Post>> {
+public class ChanReaderRequest extends JsonReaderRequest<ChanReaderRequest.ChanReaderResponse> {
+    private static final String TAG = "ChanReaderRequest";
+
     private Loadable loadable;
     private List<Post> cached;
+    private Post op;
 
-    private ChanReaderRequest(String url, Listener<List<Post>> listener, ErrorListener errorListener) {
+    private ChanReaderRequest(String url, Listener<ChanReaderResponse> listener, ErrorListener errorListener) {
         super(url, listener, errorListener);
     }
 
-    public static ChanReaderRequest newInstance(Loadable loadable, List<Post> cached, Listener<List<Post>> listener, ErrorListener errorListener) {
+    public static ChanReaderRequest newInstance(Loadable loadable, List<Post> cached, Listener<ChanReaderResponse> listener, ErrorListener errorListener) {
         String url;
 
         if (loadable.isThreadMode()) {
@@ -67,7 +68,7 @@ public class ChanReaderRequest extends JsonReaderRequest<List<Post>> {
     }
 
     @Override
-    public List<Post> readJson(JsonReader reader) throws Exception {
+    public ChanReaderResponse readJson(JsonReader reader) throws Exception {
         List<Post> list;
 
         if (loadable.isThreadMode()) {
@@ -81,11 +82,14 @@ public class ChanReaderRequest extends JsonReaderRequest<List<Post>> {
         return processPosts(list);
     }
 
-    private List<Post> processPosts(List<Post> serverList) throws Exception {
+    private ChanReaderResponse processPosts(List<Post> serverList) throws Exception {
-        List<Post> totalList = new ArrayList<>(serverList.size());
+        ChanReaderResponse response = new ChanReaderResponse();
+        response.posts = new ArrayList<>(serverList.size());
+        response.op = op;
 
         if (cached.size() > 0) {
-            totalList.addAll(cached);
+            // Add all posts that were parsed before
+            response.posts.addAll(cached);
 
             // If there's a cached post but it's not in the list received from the server, mark it as deleted
             if (loadable.isThreadMode()) {
@@ -99,7 +103,7 @@ public class ChanReaderRequest extends JsonReaderRequest<List<Post>> {
                         }
                     }
 
-                    cache.deleted = !serverHas;
+                    cache.deleted.set(!serverHas);
                 }
             }
 
@@ -115,68 +119,19 @@ public class ChanReaderRequest extends JsonReaderRequest<List<Post>> {
                     }
                 }
 
-                // serverPost is not in finalList
                 if (!known) {
-                    totalList.add(post);
+                    response.posts.add(post);
                 }
             }
-
-            // Replace OPs
-            if (totalList.get(0).isOP && serverList.size() > 0 && serverList.get(0).isOP) {
-                totalList.set(0, serverList.get(0));
-            }
-
-            // Sort if it got out of order due to posts disappearing/reappearing
-            /*if (loadable.isThreadMode()) {
-                Collections.sort(totalList, new Comparator<Post>() {
-                    @Override
-                    public int compare(Post lhs, Post rhs) {
-                        return lhs.time == rhs.time ? 0 : (lhs.time < rhs.time ? -1 : 1);
-                    }
-                });
-            }*/
-
         } else {
-            totalList.addAll(serverList);
+            response.posts.addAll(serverList);
-        }
-
-        Set<Integer> postsReplyingToDeleted = new HashSet<>();
-        for (Post post : totalList) {
-            if (!post.deleted) {
-                post.repliesFrom.clear();
-
-                for (Post other : totalList) {
-                    if (other.repliesTo.contains(post.no) && !other.deleted) {
-                        post.repliesFrom.add(other.no);
-                    }
-                }
-            } else {
-                post.repliesTo.clear();
-
-                for (int no : post.repliesFrom) {
-                    postsReplyingToDeleted.add(no);
-                }
-
-                post.repliesFrom.clear();
-            }
         }
 
-        for (int no : postsReplyingToDeleted) {
+        for (Post post : response.posts) {
-            for (Post post : totalList) {
+            post.isSavedReply.set(Chan.getDatabaseManager().isSavedReply(post.board, post.no));
-                if (post.no == no) {
-                    if (!post.finish()) {
-                        throw new IOException("Incorrect data about post received.");
-                    }
-                    break;
-                }
-            }
         }
 
-        for (Post post : totalList) {
+        return response;
-            post.isSavedReply = Chan.getDatabaseManager().isSavedReply(post.board, post.no);
-        }
-
-        return totalList;
     }
 
     private List<Post> loadThread(JsonReader reader) throws Exception {
@@ -191,7 +146,10 @@ public class ChanReaderRequest extends JsonReaderRequest<List<Post>> {
                 // Thread array
                 while (reader.hasNext()) {
                     // Thread object
-                    list.add(readPostObject(reader));
+                    Post post = readPostObject(reader);
+                    if (post != null) {
+                        list.add(post);
+                    }
                 }
                 reader.endArray();
             } else {
@@ -216,7 +174,10 @@ public class ChanReaderRequest extends JsonReaderRequest<List<Post>> {
                     reader.beginArray(); // Threads array
 
                     while (reader.hasNext()) {
-                        list.add(readPostObject(reader));
+                        Post post = readPostObject(reader);
+                        if (post != null) {
+                            list.add(post);
+                        }
                     }
 
                     reader.endArray();
@@ -324,28 +285,28 @@ public class ChanReaderRequest extends JsonReaderRequest<List<Post>> {
                     break;
                 default:
                     // Unknown/ignored key
-                    //                log("Unknown/ignored key: " + key + ".");
                     reader.skipValue();
                     break;
             }
         }
         reader.endObject();
 
+        if (post.resto == 0) {
+            // Update OP fields later on the main thread
+            op = new Post();
+            op.closed = post.closed;
+            op.archived = post.archived;
+            op.sticky = post.sticky;
+            op.replies = post.replies;
+            op.images = post.images;
+            op.uniqueIps = post.uniqueIps;
+        }
+
         Post cached = null;
         for (Post item : this.cached) {
             if (item.no == post.no) {
                 cached = item;
 
-                if (post.resto == 0) {
-                    // Update OP fields
-                    cached.sticky = post.sticky;
-                    cached.closed = post.closed;
-                    cached.archived = post.archived;
-                    cached.replies = post.replies;
-                    cached.images = post.images;
-                    cached.uniqueIps = post.uniqueIps;
-                }
-
                 break;
             }
         }
@@ -354,10 +315,18 @@ public class ChanReaderRequest extends JsonReaderRequest<List<Post>> {
             return cached;
         } else {
             if (!post.finish()) {
-                throw new IOException("Incorrect data about post received.");
+                Logger.e(TAG, "Incorrect data about post received for post " + post.no);
+                return null;
+            } else {
+                return post;
             }
-
-            return post;
         }
     }
+
+    public static class ChanReaderResponse {
+        // Op Post that is created new each time.<br>
+        // Used to later copy members like image count to the real op on the main thread.
+        public Post op;
+        public List<Post> posts;
+    }
 }

Clover/app/src/main/java/org/floens/chan/core/watch/PinWatcher.java

@@ -147,7 +147,7 @@ public class PinWatcher implements ChanLoader.ChanLoaderCallback {
         for (Post item : thread.posts) {
 //            saved.title = pin.loadable.title;
 
-            if (item.isSavedReply) {
+            if (item.isSavedReply.get()) {
                 savedReplies.add(item);
             }
         }

Clover/app/src/main/java/org/floens/chan/database/DatabaseManager.java

@@ -47,11 +47,12 @@ public class DatabaseManager {
 
     private final DatabaseHelper helper;
 
-    private List<SavedReply> savedReplies = new ArrayList<>();
+    private final Object savedRepliesLock = new Object();
-    private HashSet<Integer> savedRepliesIds = new HashSet<>();
+    private final List<SavedReply> savedReplies = new ArrayList<>();
+    private final HashSet<Integer> savedRepliesIds = new HashSet<>();
 
-    private List<ThreadHide> threadHides = new ArrayList<>();
+    private final List<ThreadHide> threadHides = new ArrayList<>();
-    private HashSet<Integer> threadHidesIds = new HashSet<>();
+    private final HashSet<Integer> threadHidesIds = new HashSet<>();
 
     public DatabaseManager(Context context) {
         helper = new DatabaseHelper(context);
@@ -60,6 +61,7 @@ public class DatabaseManager {
 
     /**
      * Save a reply to the savedreply table.
+     * Threadsafe.
      *
      * @param saved the {@link SavedReply} to save
      */
@@ -70,22 +72,27 @@ public class DatabaseManager {
             Logger.e(TAG, "Error saving reply", e);
         }
 
-        savedReplies.add(saved);
+        synchronized (savedRepliesLock) {
-        savedRepliesIds.add(saved.no);
+            savedReplies.add(saved);
+            savedRepliesIds.add(saved.no);
+        }
     }
 
     /**
      * Searches a saved reply. This is done through caching members, no database lookups.
+     * Threadsafe.
      *
      * @param board board for the reply to search
      * @param no    no for the reply to search
      * @return A {@link SavedReply} that matches {@code board} and {@code no}, or {@code null}
      */
     public SavedReply getSavedReply(String board, int no) {
-        if (savedRepliesIds.contains(no)) {
+        synchronized (savedRepliesLock) {
-            for (SavedReply r : savedReplies) {
+            if (savedRepliesIds.contains(no)) {
-                if (r.no == no && r.board.equals(board)) {
+                for (SavedReply r : savedReplies) {
-                    return r;
+                    if (r.no == no && r.board.equals(board)) {
+                        return r;
+                    }
                 }
             }
         }
@@ -95,6 +102,7 @@ public class DatabaseManager {
 
     /**
      * Searches if a saved reply exists. This is done through caching members, no database lookups.
+     * Threadsafe.
      *
      * @param board board for the reply to search
      * @param no    no for the reply to search
@@ -324,15 +332,20 @@ public class DatabaseManager {
         loadThreadHides();
     }
 
+    /**
+     * Threadsafe.
+     */
     private void loadSavedReplies() {
         try {
             trimTable(helper.savedDao, "savedreply", SAVED_REPLY_TRIM_TRIGGER, SAVED_REPLY_TRIM_COUNT);
 
-            savedReplies.clear();
+            synchronized (savedRepliesLock) {
-            savedReplies.addAll(helper.savedDao.queryForAll());
+                savedReplies.clear();
-            savedRepliesIds.clear();
+                savedReplies.addAll(helper.savedDao.queryForAll());
-            for (SavedReply reply : savedReplies) {
+                savedRepliesIds.clear();
-                savedRepliesIds.add(reply.no);
+                for (SavedReply reply : savedReplies) {
+                    savedRepliesIds.add(reply.no);
+                }
             }
         } catch (SQLException e) {
             Logger.e(TAG, "Error loading saved replies", e);

Clover/app/src/main/java/org/floens/chan/ui/cell/PostCell.java

@@ -273,7 +273,7 @@ public class PostCell extends RelativeLayout implements PostCellInterface, PostL
 
         if (highlighted) {
             setBackgroundColor(theme.highlightedColor);
-        } else if (post.isSavedReply) {
+        } else if (post.isSavedReply.get()) {
             setBackgroundColor(theme.savedReplyColor);
         } else if (threadMode) {
             setBackgroundResource(0);
@@ -320,7 +320,7 @@ public class PostCell extends RelativeLayout implements PostCellInterface, PostL
             iconsSpannable = PostHelper.addIcon(iconsSpannable, PostHelper.closedIcon, iconsTextSize);
         }
 
-        if (post.deleted) {
+        if (post.deleted.get()) {
             iconsSpannable = PostHelper.addIcon(iconsSpannable, PostHelper.trashIcon, iconsTextSize);
         }