From 9f62ab798cbb15771151b06d2ee2c2cd41da1170 Mon Sep 17 00:00:00 2001
From: Konrad Kollnig <5175206+kasnder@users.noreply.github.com>
Date: Sat, 22 Aug 2026 08:13:45 +0200
Subject: [PATCH] Synchronize TrackerBlocklist access across packet and UI
 threads

Readers (blocked/blockedTracker/getSubset) ran unsynchronized on native
JNI packet threads while writers on the UI thread mutated the same plain
HashSet values, risking ConcurrentModificationException inside the VPN
packet path and torn reads. The lazy singleton in getInstance could also
split-brain into two instances under concurrent first calls, leaving an
orphan instance serving stale decisions. Make all entry points mutually
exclusive; no behavior change.
---
 .../missioncontrol/data/TrackerBlocklist.java | 15 ++++---
 .../data/TrackerBlocklistTest.java            | 45 +++++++++++++++++++
 2 files changed, 53 insertions(+), 7 deletions(-)

diff --git a/app/src/main/java/net/kollnig/missioncontrol/data/TrackerBlocklist.java b/app/src/main/java/net/kollnig/missioncontrol/data/TrackerBlocklist.java
index de90b806d..3d47587ac 100644
--- a/app/src/main/java/net/kollnig/missioncontrol/data/TrackerBlocklist.java
+++ b/app/src/main/java/net/kollnig/missioncontrol/data/TrackerBlocklist.java
@@ -58,7 +58,8 @@ private TrackerBlocklist(Context c) {
      * @return The current instance of the TrackerBlocklist, if none, a new instance
      *         is created.
      */
-    public static TrackerBlocklist getInstance(Context c) {
+    // Called from both native packet threads and UI threads.
+    public static synchronized TrackerBlocklist getInstance(Context c) {
         if (instance == null)
             instance = new TrackerBlocklist(c);
 
@@ -85,7 +86,7 @@ public static String getBlockingKey(Tracker t) {
      *
      * @param c Context
      */
-    public void loadSettings(Context c) {
+    public synchronized void loadSettings(Context c) {
         SharedPreferences prefs = c.getSharedPreferences(PREF_BLOCKLIST, Context.MODE_PRIVATE);
         Set<String> set = prefs.getStringSet(SHARED_PREFS_BLOCKLIST_APPS_KEY, null);
         PackageUidResolver resolver = new PackageUidResolver() {
@@ -226,14 +227,14 @@ public Set<Integer> getBlocklist() {
      * @param uid Uid of the app
      * @return Information about what specific trackers are blocked
      */
-    public Set<String> getSubset(int uid) {
+    public synchronized Set<String> getSubset(int uid) {
         return blockmap.get(uid);
     }
 
     /**
      * Completely clear blocklist.
      */
-    public void clear() {
+    public synchronized void clear() {
         blockmap.clear();
     }
 
@@ -242,7 +243,7 @@ public void clear() {
      *
      * @param uid Uid of app
      */
-    public void clear(int uid) {
+    public synchronized void clear(int uid) {
         blockmap.remove(uid);
     }
 
@@ -303,7 +304,7 @@ public synchronized void unblock(int uid, Tracker t) {
      * @param key Key of the tracker
      * @return Whether access to this tracker is blocked
      */
-    public boolean blocked(int uid, String key) {
+    public synchronized boolean blocked(int uid, String key) {
         Set<String> trackers = this.getSubset(uid);
         if (trackers == null) {
             return true;
@@ -319,7 +320,7 @@ public boolean blocked(int uid, String key) {
      * @param t   Tracker
      * @return Whether access to this tracker is blocked
      */
-    public boolean blockedTracker(int uid, Tracker t) {
+    public synchronized boolean blockedTracker(int uid, Tracker t) {
         return blocked(uid, t.category)
                 && blocked(uid, getBlockingKey(t));
     }
diff --git a/app/src/test/java/net/kollnig/missioncontrol/data/TrackerBlocklistTest.java b/app/src/test/java/net/kollnig/missioncontrol/data/TrackerBlocklistTest.java
index 08f7295cf..4e282bbb8 100644
--- a/app/src/test/java/net/kollnig/missioncontrol/data/TrackerBlocklistTest.java
+++ b/app/src/test/java/net/kollnig/missioncontrol/data/TrackerBlocklistTest.java
@@ -21,6 +21,10 @@
 import org.junit.Before;
 import org.junit.Test;
 
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
 public class TrackerBlocklistTest {
     private static final int UID = 1001;
 
@@ -202,4 +206,45 @@ public void blockedTrackerRequiresBothCategoryAndKeyBlocked() {
         blocklist.block(UID, tracker);
         assertTrue(blocklist.blockedTracker(UID, tracker));
     }
+
+    @Test
+    public void concurrentReadersAndWritersDoNotThrow() throws Exception {
+        TrackerBlocklist blocklist = TrackerBlocklist.getInstance(null);
+        int uidCount = 8;
+        for (int uid = 0; uid < uidCount; uid++)
+            blocklist.ensureDefaults(uid, false);
+
+        final int iterations = 1000;
+        List<Throwable> errors = Collections.synchronizedList(new ArrayList<>());
+
+        Runnable writer = () -> {
+            try {
+                for (int i = 0; i < iterations; i++) {
+                    blocklist.applyStrictModeToAll(i % 2 == 0);
+                    blocklist.unblock(0, "Advertising | Writer");
+                    blocklist.block(0, "Advertising | Writer");
+                }
+            } catch (Throwable t) {
+                errors.add(t);
+            }
+        };
+        Runnable reader = () -> {
+            try {
+                for (int i = 0; i < iterations; i++)
+                    for (int uid = 0; uid < uidCount; uid++)
+                        blocklist.blocked(uid, "Content");
+            } catch (Throwable t) {
+                errors.add(t);
+            }
+        };
+
+        Thread[] threads = {new Thread(writer), new Thread(writer),
+                new Thread(reader), new Thread(reader)};
+        for (Thread thread : threads)
+            thread.start();
+        for (Thread thread : threads)
+            thread.join(30000);
+
+        assertTrue(errors.toString(), errors.isEmpty());
+    }
 }