Skip to content

Conversation

@alperozturk96
Copy link
Contributor

@alperozturk96 alperozturk96 commented Jul 15, 2025

Issue

DNSCache used @Synchronized on all public methods to protect access to a MutableMap.

All threads must acquire the lock, even for read operations like isIPV6First() or lookup()

Methods like lookup() perform DNS resolution, which can be slow. Holding the @Synchronized lock this increases the risk of blocking critical threads like the UI or Android system event receivers.

Changes

Replaced MutableMap with ConcurrentHashMap to enable lock-free concurrent access.
Removed unnecessary @Synchronized annotations from all methods.
UseConcurrentHashMap.compute(), ensuring atomic read–modify–write operations without external locking.

Issue Logs

Logs:
"main" tid=1 Blocked
 at com.nextcloud.common.DNSCache.clear (unavailable:2)
 at [com.owncloud.android](http://com.owncloud.android/).utils.ReceiversHelper$1.onReceive ([ReceiversHelper.java:49](http://receivershelper.java:49/))
 at [android.app](http://android.app/).LoadedApk$ReceiverDispatcher$Args.lambda$getRunnable$0 ([LoadedApk.java:1821](http://loadedapk.java:1821/))
 at [android.app](http://android.app/).LoadedApk$ReceiverDispatcher$Args.$r8$lambda$gDuJqgxY6Zb-ifyeubKeivTLAwk (unavailable)
 at [android.app](http://android.app/).LoadedApk$ReceiverDispatcher$Args$$ExternalSyntheticLambda0.run (unavailable:2)
 at android.os.Handler.handleCallback ([Handler.java:1000](http://handler.java:1000/))
 at android.os.Handler.dispatchMessage ([Handler.java:104](http://handler.java:104/))
 at android.os.Looper.loopOnce ([Looper.java:242](http://looper.java:242/))
 at android.os.Looper.loop ([Looper.java:362](http://looper.java:362/))
 at [android.app](http://android.app/).ActivityThread.main ([ActivityThread.java:8393](http://activitythread.java:8393/))
 at java.lang.reflect.Method.invoke (Native method)
 at [com.android](http://com.android/).internal.os.RuntimeInit$MethodAndArgsCaller.run ([RuntimeInit.java:552](http://runtimeinit.java:552/))
 at [com.android](http://com.android/).internal.os.ZygoteInit.main ([ZygoteInit.java:992](http://zygoteinit.java:992/))

"androidx.work-3" tid=41 Native
 #00  pc 0x00000000000ec824  /apex/com.android.runtime/lib64/bionic/libc.so (read+4)
 #01  pc 0x00000000000f8990  /apex/com.android.runtime/lib64/bionic/libc.so (__sread+48)
 #02  pc 0x00000000000f887c  /apex/com.android.runtime/lib64/bionic/libc.so (__srefill+268)

"ReferenceQueueDaemon" tid=6 Waiting
 at java.lang.Object.wait (Native method)
 at java.lang.Object.wait ([Object.java:405](http://object.java:405/))

"FinalizerDaemon" tid=7 Waiting
 at java.lang.Object.wait (Native method)
 at java.lang.Object.wait ([Object.java:405](http://object.java:405/))

"MultiThreadedHttpConnectionManager cleanup" tid=51 Waiting
 at java.lang.Object.wait (Native method)

"androidx.work-1" tid=38 Waiting
 at jdk.internal.misc.Unsafe.park (Native method)
 at java.util.concurrent.locks.LockSupport.park ([LockSupport.java:341](http://locksupport.java:341/))
 at java.lang.Object.wait ([Object.java:405](http://object.java:405/))

...

Signed-off-by: alperozturk <[email protected]>
@alperozturk96 alperozturk96 changed the title BugFix - DNSCache Thread Lock BugFix - DNSCache Lock Jul 15, 2025
@github-actions
Copy link
Contributor

SpotBugs

CategoryBaseNew
Bad practice3434
Correctness3434
Dodgy code2626
Internationalization66
Malicious code vulnerability4949
Multithreaded correctness1111
Performance88
Total168168

@codecov
Copy link

codecov bot commented Jul 15, 2025

Codecov Report

Attention: Patch coverage is 30.76923% with 9 lines in your changes missing coverage. Please review.

Project coverage is 43.01%. Comparing base (ccc7ae5) to head (97125f6).
Report is 12 commits behind head on master.

Files with missing lines Patch % Lines
...ary/src/main/java/com/nextcloud/common/DNSCache.kt 30.76% 9 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1770      +/-   ##
============================================
- Coverage     43.06%   43.01%   -0.05%     
  Complexity      987      987              
============================================
  Files           235      235              
  Lines          8569     8569              
  Branches       1123     1124       +1     
============================================
- Hits           3690     3686       -4     
- Misses         4361     4365       +4     
  Partials        518      518              
Files with missing lines Coverage Δ
...ary/src/main/java/com/nextcloud/common/DNSCache.kt 43.75% <30.76%> (-8.34%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions
Copy link
Contributor

SpotBugs

CategoryBaseNew
Bad practice3434
Correctness3434
Dodgy code2626
Internationalization66
Malicious code vulnerability4949
Multithreaded correctness1111
Performance88
Total168168

Signed-off-by: alperozturk <[email protected]>
@github-actions
Copy link
Contributor

SpotBugs

CategoryBaseNew
Bad practice3434
Correctness3434
Dodgy code2626
Internationalization66
Malicious code vulnerability4949
Multithreaded correctness1111
Performance88
Total168168

@alperozturk96 alperozturk96 merged commit cee7b9f into master Jul 18, 2025
19 of 21 checks passed
@alperozturk96 alperozturk96 deleted the bugfix/dns-cache-blocking-thread branch July 18, 2025 06:52
@alperozturk96
Copy link
Contributor Author

/backport to stable-2.21

@nextcloud nextcloud deleted a comment from backportbot bot Jul 18, 2025
@nextcloud nextcloud deleted a comment from tobiasKaminsky Jul 18, 2025
@alperozturk96
Copy link
Contributor Author

/backport to stable-2.21

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants