Fix lock deadlocks, production NPE, connection leaks, and log typo

- Wrap all lock.readLock/writeLock operations in try-finally blocks in
  ServiceSinkhole to prevent permanent deadlocks if exceptions occur
  while locks are held (e.g. during DB access in blockKnownTracker).
  Also fixes prepareHostsBlocked where unlock() ran unconditionally
  even when the lock was never acquired due to early return.
- Replace disabled assert + chained NPE in tracker logcat logging:
  assert is a no-op in production Android, and ipToHost.get(daddr)
  can return null, causing a crash when log_logcat is enabled.
- Fix copy-paste bug in ActivityMain.onActivityResult logging
  requestCode instead of resultCode.
- Add HttpURLConnection.disconnect() in finally blocks in Common.kt
  and Util.java to prevent socket leaks on repeated network calls.
- Move svg.renderToPicture() off the main thread in CountriesFragment;
  it was executing inside View.post() which runs on the UI thread.

https://claude.ai/code/session_011HzyMPo2gd34cnXG4EWLYj

Author: claude

app/src/main/java/eu/faircode/netguard/ActivityMain.java

@@ -571,7 +571,7 @@ public void onDestroy() {
 
     @Override
     protected void onActivityResult(int requestCode, int resultCode, final Intent data) {
-        Log.i(TAG, "onActivityResult request=" + requestCode + " result=" + requestCode + " ok="
+        Log.i(TAG, "onActivityResult request=" + requestCode + " result=" + resultCode + " ok="
                 + (resultCode == RESULT_OK));
         Util.logExtras(data);
 

app/src/main/java/eu/faircode/netguard/ServiceSinkhole.java

@@ -1689,12 +1689,15 @@ private void startNative(final ParcelFileDescriptor vpn, List<Rule> listAllowed,
             prepareForwarding();
         } else {
             lock.writeLock().lock();
-            mapUidAllowed.clear();
-            mapUidKnown.clear();
-            mapHostsBlocked.clear();
-            mapUidIPFilters.clear();
-            mapForward.clear();
-            lock.writeLock().unlock();
+            try {
+                mapUidAllowed.clear();
+                mapUidKnown.clear();
+                mapHostsBlocked.clear();
+                mapUidIPFilters.clear();
+                mapForward.clear();
+            } finally {
+                lock.writeLock().unlock();
+            }
         }
 
         if (log || log_app || filter) {
@@ -1760,26 +1763,30 @@ private void stopNative(ParcelFileDescriptor vpn) {
 
     private void unprepare() {
         lock.writeLock().lock();
-        mapUidAllowed.clear();
-        mapUidKnown.clear();
-        mapHostsBlocked.clear();
-        mapUidIPFilters.clear();
-        mapForward.clear();
-        lock.writeLock().unlock();
+        try {
+            mapUidAllowed.clear();
+            mapUidKnown.clear();
+            mapHostsBlocked.clear();
+            mapUidIPFilters.clear();
+            mapForward.clear();
+        } finally {
+            lock.writeLock().unlock();
+        }
     }
 
     private void prepareUidAllowed(List<Rule> listAllowed, List<Rule> listRule) {
         lock.writeLock().lock();
+        try {
+            mapUidAllowed.clear();
+            for (Rule rule : listAllowed)
+                mapUidAllowed.put(rule.uid, true);
 
-        mapUidAllowed.clear();
-        for (Rule rule : listAllowed)
-            mapUidAllowed.put(rule.uid, true);
-
-        mapUidKnown.clear();
-        for (Rule rule : listRule)
-            mapUidKnown.put(rule.uid, rule.uid);
-
-        lock.writeLock().unlock();
+            mapUidKnown.clear();
+            for (Rule rule : listRule)
+                mapUidKnown.put(rule.uid, rule.uid);
+        } finally {
+            lock.writeLock().unlock();
+        }
     }
 
     public static void prepareHostsBlocked(Context c) {
@@ -1805,27 +1812,31 @@ public static void prepareHostsBlocked(Context c) {
             }
 
             lock.writeLock().lock();
-            mapHostsBlocked.clear();
+            try {
+                mapHostsBlocked.clear();
 
-            int count = 0;
-            br = new BufferedReader(is);
-            String line;
-            while ((line = br.readLine()) != null) {
-                int hash = line.indexOf('#');
-                if (hash >= 0)
-                    line = line.substring(0, hash);
-                line = line.trim();
-                if (line.length() > 0) {
-                    String[] words = line.split("\\s+");
-                    if (words.length == 2) {
-                        count++;
-                        mapHostsBlocked.put(words[1], true);
-                    } else
-                        Log.i(TAG, "Invalid hosts file line: " + line);
-                }
-            }
-            mapHostsBlocked.put("test.netguard.me", true);
-            Log.i(TAG, count + " hosts read");
+                int count = 0;
+                br = new BufferedReader(is);
+                String line;
+                while ((line = br.readLine()) != null) {
+                    int hash = line.indexOf('#');
+                    if (hash >= 0)
+                        line = line.substring(0, hash);
+                    line = line.trim();
+                    if (line.length() > 0) {
+                        String[] words = line.split("\\s+");
+                        if (words.length == 2) {
+                            count++;
+                            mapHostsBlocked.put(words[1], true);
+                        } else
+                            Log.i(TAG, "Invalid hosts file line: " + line);
+                    }
+                }
+                mapHostsBlocked.put("test.netguard.me", true);
+                Log.i(TAG, count + " hosts read");
+            } finally {
+                lock.writeLock().unlock();
+            }
         } catch (IOException ex) {
             Log.e(TAG, ex.toString() + "\n" + Log.getStackTraceString(ex));
         } finally {
@@ -1843,8 +1854,6 @@ public static void prepareHostsBlocked(Context c) {
                 }
         }
 
-        lock.writeLock().unlock();
-
         // Reload TrackerList to ensure it stays in sync with updated hosts
         TrackerList.reloadTrackerData(c);
     }
@@ -1853,11 +1862,11 @@ private void prepareUidIPFilters(String dname) {
         SharedPreferences lockdown = getSharedPreferences("lockdown", Context.MODE_PRIVATE);
 
         lock.writeLock().lock();
+        try {
+            if (dname == null) // reset mechanism, called from startNative()
+                mapUidIPFilters.clear();
 
-        if (dname == null) // reset mechanism, called from startNative()
-            mapUidIPFilters.clear();
-
-        try (Cursor cursor = DatabaseHelper.getInstance(ServiceSinkhole.this).getAccessDns(dname)) {
+            try (Cursor cursor = DatabaseHelper.getInstance(ServiceSinkhole.this).getAccessDns(dname)) {
             int colUid = cursor.getColumnIndex("uid");
             int colVersion = cursor.getColumnIndex("version");
             int colProtocol = cursor.getColumnIndex("protocol");
@@ -1930,13 +1939,15 @@ private void prepareUidIPFilters(String dname) {
                 }
             }
         }
-
-        lock.writeLock().unlock();
+        } finally {
+            lock.writeLock().unlock();
+        }
     }
 
     private void prepareForwarding() {
         lock.writeLock().lock();
-        mapForward.clear();
+        try {
+            mapForward.clear();
 
         SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(this);
         if (prefs.getBoolean("filter", true)) {
@@ -1983,7 +1994,9 @@ private void prepareForwarding() {
                 Log.i(TAG, "DoH Forward " + dnsFwd);
             }
         }
-        lock.writeLock().unlock();
+        } finally {
+            lock.writeLock().unlock();
+        }
     }
 
     private boolean isLockedDown(boolean metered) {
@@ -2158,68 +2171,69 @@ public static void clearTrackerCaches() {
     private Allowed isAddressAllowed(Packet packet) {
         SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(this);
 
+        Allowed allowed = null;
         lock.readLock().lock();
+        try {
+            packet.allowed = false;
+            if (prefs.getBoolean("filter", true)) {
+                // https://android.googlesource.com/platform/system/core/+/master/include/private/android_filesystem_config.h
+                if (packet.protocol == 17 /* UDP */ && !prefs.getBoolean("filter_udp", true)) {
+                    // Allow unfiltered UDP
+                    packet.allowed = true;
+                    Log.i(TAG, "Allowing UDP " + packet);
+                } else if ((packet.uid < 2000) &&
+                        !mapUidKnown.containsKey(packet.uid) && isSupported(packet.protocol)) {
+                    // Allow unknown (system) traffic
+                    packet.allowed = true;
+                    Log.w(TAG, "Allowing unknown system " + packet);
+                } else if (packet.uid == Process.myUid()) {
+                    // Allow self
+                    packet.allowed = true;
+                    Log.w(TAG, "Allowing self " + packet);
+                } else {
+                    boolean filtered = false;
 
-        packet.allowed = false;
-        if (prefs.getBoolean("filter", true)) {
-            // https://android.googlesource.com/platform/system/core/+/master/include/private/android_filesystem_config.h
-            if (packet.protocol == 17 /* UDP */ && !prefs.getBoolean("filter_udp", true)) {
-                // Allow unfiltered UDP
-                packet.allowed = true;
-                Log.i(TAG, "Allowing UDP " + packet);
-            } else if ((packet.uid < 2000) &&
-                    !mapUidKnown.containsKey(packet.uid) && isSupported(packet.protocol)) {
-                // Allow unknown (system) traffic
-                packet.allowed = true;
-                Log.w(TAG, "Allowing unknown system " + packet);
-            } else if (packet.uid == Process.myUid()) {
-                // Allow self
-                packet.allowed = true;
-                Log.w(TAG, "Allowing self " + packet);
-            } else {
-                boolean filtered = false;
+                    if (packet.data != null && !packet.data.isEmpty())
+                        Log.d(TAG, "Found SNI in isAddressAllowed: " + packet.data);
 
-                if (packet.data != null && !packet.data.isEmpty())
-                    Log.d(TAG, "Found SNI in isAddressAllowed: " + packet.data);
+                    // Check if tracker is known
+                    // In minimal mode (including TC Slim), always enable blocking
+                    if (blockKnownTracker(packet.daddr, packet.uid)) {
+                        filtered = true;
+                        packet.allowed = false;
+                    }
 
-                // Check if tracker is known
-                // In minimal mode (including TC Slim), always enable blocking
-                if (blockKnownTracker(packet.daddr, packet.uid)) {
-                    filtered = true;
-                    packet.allowed = false;
-                }
+                    InternetBlocklist internetBlocklist = InternetBlocklist.getInstance(ServiceSinkhole.this);
+                    if (internetBlocklist.blockedInternet(packet.uid)) {
+                        filtered = true;
+                        packet.allowed = false;
+                    }
 
-                InternetBlocklist internetBlocklist = InternetBlocklist.getInstance(ServiceSinkhole.this);
-                if (internetBlocklist.blockedInternet(packet.uid)) {
-                    filtered = true;
-                    packet.allowed = false;
+                    if (!filtered)
+                        packet.allowed = true;
                 }
-
-                if (!filtered)
-                    packet.allowed = true;
             }
-        }
 
-        // Block DNS-over-TLS (DoT) on port 853 to prevent bypassing DNS filtering
-        if (packet.allowed && packet.dport == 853 && prefs.getBoolean("block_dot", true)) {
-            Log.i(TAG, "Blocking DoT " + packet);
-            packet.allowed = false;
-        }
+            // Block DNS-over-TLS (DoT) on port 853 to prevent bypassing DNS filtering
+            if (packet.allowed && packet.dport == 853 && prefs.getBoolean("block_dot", true)) {
+                Log.i(TAG, "Blocking DoT " + packet);
+                packet.allowed = false;
+            }
 
-        Allowed allowed = null;
-        if (packet.allowed)
-            if (mapForward.containsKey(packet.dport)) {
-                Forward fwd = mapForward.get(packet.dport);
-                if (fwd.ruid == packet.uid) {
+            if (packet.allowed)
+                if (mapForward.containsKey(packet.dport)) {
+                    Forward fwd = mapForward.get(packet.dport);
+                    if (fwd.ruid == packet.uid) {
+                        allowed = new Allowed();
+                    } else {
+                        allowed = new Allowed(fwd.raddr, fwd.rport);
+                        packet.data = "> " + fwd.raddr + "/" + fwd.rport;
+                    }
+                } else
                     allowed = new Allowed();
-                } else {
-                    allowed = new Allowed(fwd.raddr, fwd.rport);
-                    packet.data = "> " + fwd.raddr + "/" + fwd.rport;
-                }
-            } else
-                allowed = new Allowed();
-
-        lock.readLock().unlock();
+        } finally {
+            lock.readLock().unlock();
+        }
 
         if (prefs.getBoolean("log", false) || prefs.getBoolean("log_app", true))
             if (packet.protocol != 6 /* TCP */ || !"".equals(packet.flags))
@@ -2379,8 +2393,11 @@ private boolean blockKnownTracker(String daddr, int uid) {
                 app = Common.getAppName(pm, uid);
                 uidToApp.put(uid, app);
             }
-            assert tracker != null;
-            Log.i("TC-Log", app + " " + daddr + " " + ipToHost.get(daddr).getOrExpired() + " " + tracker.getName());
+            if (tracker != null) {
+                Expiring<String> host = ipToHost.get(daddr);
+                String hostName = (host != null) ? host.getOrExpired() : null;
+                Log.i("TC-Log", app + " " + daddr + " " + hostName + " " + tracker.getName());
+            }
         } else {
             if (tracker != NO_TRACKER) {
                 boolean blockedByGranularRule = false;

app/src/main/java/eu/faircode/netguard/Util.java

@@ -586,9 +586,10 @@ public static String getOrganization(String ip) throws Exception {
                 return mapIPOrganization.get(ip);
         }
         BufferedReader reader = null;
+        HttpURLConnection connection = null;
         try {
             URL url = new URL("https://ipinfo.io/" + ip + "/org");
-            HttpURLConnection connection = (HttpURLConnection) url.openConnection();
+            connection = (HttpURLConnection) url.openConnection();
             connection.setRequestProperty("Accept-Encoding", "gzip");
             connection.setRequestMethod("GET");
             connection.setReadTimeout(15 * 1000);
@@ -607,6 +608,8 @@ public static String getOrganization(String ip) throws Exception {
         } finally {
             if (reader != null)
                 reader.close();
+            if (connection != null)
+                connection.disconnect();
         }
     }
 

app/src/main/java/net/kollnig/missioncontrol/Common.kt

@@ -43,27 +43,30 @@ object Common {
      */
     @JvmStatic
     fun fetch(url: String?): String? {
+        var conn: HttpURLConnection? = null
         try {
             val html = StringBuilder()
 
-            val conn = (URL(url)).openConnection() as HttpURLConnection
+            conn = (URL(url)).openConnection() as HttpURLConnection
             conn.setRequestProperty("Accept-Encoding", "gzip")
             conn.setConnectTimeout(5000)
-            val `in`: BufferedReader?
-            if ("gzip" == conn.getContentEncoding()) `in` =
+            val reader: BufferedReader = if ("gzip" == conn.getContentEncoding())
                 BufferedReader(InputStreamReader(GZIPInputStream(conn.getInputStream())))
-            else `in` = BufferedReader(InputStreamReader(conn.getInputStream()))
+            else
+                BufferedReader(InputStreamReader(conn.getInputStream()))
 
-            var str: String?
-            while ((`in`.readLine().also { str = it }) != null) html.append(str)
-
-            `in`.close()
+            reader.use { r ->
+                var str: String?
+                while ((r.readLine().also { str = it }) != null) html.append(str)
+            }
 
             return html.toString()
         } catch (e: IOException) {
             return null
         } catch (e: OutOfMemoryError) {
             return null
+        } finally {
+            conn?.disconnect()
         }
     }
 

app/src/main/java/net/kollnig/missioncontrol/details/CountriesFragment.java

@@ -163,8 +163,8 @@ private void showCountriesMap(ProgressBar pbLoading, ImageView mv, TextView txtF
             String countries = TextUtils.join(",#", hostCountriesCount.keySet());
             renderOptions.css(String.format("#%s { fill: #B71C1C; }", countries.toUpperCase()));
 
+            Picture picture = svg.renderToPicture(renderOptions);
             mv.post(() -> {
-                Picture picture = svg.renderToPicture(renderOptions);
                 mv.setImageDrawable(new PictureDrawable(picture));
                 pbLoading.setVisibility(View.GONE);
             });