fix(markdown): Fix concurrency issues

Signed-off-by: Stefan Niedermann <[email protected]>
stefan-niedermann · Feb 2, 2024 · b9968d7 · b9968d7
1 parent a885eda
commit b9968d7
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 14 deletions.
diff --git a/markdown/src/main/java/it/niedermann/android/markdown/markwon/MarkwonMarkdownViewer.java b/markdown/src/main/java/it/niedermann/android/markdown/markwon/MarkwonMarkdownViewer.java
@@ -133,7 +133,7 @@ private Markwon.Builder createMarkwonBuilder(@NonNull Context context,
                 }));
 
         if (enableMentions) {
-//            return builder.usePlugin(MentionsPlugin.create(getContext(), (int) getTextSize(), color));
+            return builder.usePlugin(MentionsPlugin.create(getContext(), (int) getTextSize(), color));
         }
 
         return builder;

diff --git a/...rc/main/java/it/niedermann/android/markdown/markwon/plugins/mentions/DisplayNameUtil.java b/...rc/main/java/it/niedermann/android/markdown/markwon/plugins/mentions/DisplayNameUtil.java
@@ -95,7 +95,7 @@ public Map<String, String> fetchDisplayNames(@NonNull Context context,
             return Collections.emptyMap();
         }
 
-        final var cachedUsers = userCache
+        final var checkedUsernames = userCache
                 .entrySet()
                 .stream()
                 .filter(entry -> potentialUserNames.contains(entry.getKey()))
@@ -106,8 +106,12 @@ public Map<String, String> fetchDisplayNames(@NonNull Context context,
                 .filter(potentialUserName -> !noUserCache.contains(potentialUserName))
                 .collect(Collectors.toUnmodifiableSet());
 
-        final var result = new ConcurrentHashMap<String, String>(cachedUsers.size() + usernamesToCheck.size());
-        result.putAll(cachedUsers);
+        if (usernamesToCheck.isEmpty()) {
+            return checkedUsernames;
+        }
+
+        final var result = new ConcurrentHashMap<String, String>(checkedUsernames.size() + usernamesToCheck.size());
+        result.putAll(checkedUsernames);
 
         final var latch = new CountDownLatch(usernamesToCheck.size());
         final var executor = this.executorFactory.createExecutor(usernamesToCheck.size());

diff --git a/...src/main/java/it/niedermann/android/markdown/markwon/plugins/mentions/MentionsPlugin.java b/...src/main/java/it/niedermann/android/markdown/markwon/plugins/mentions/MentionsPlugin.java
@@ -1,5 +1,7 @@
 package it.niedermann.android.markdown.markwon.plugins.mentions;
 
+import static java.util.function.Predicate.not;
+
 import android.content.Context;
 import android.graphics.drawable.Drawable;
 import android.text.Spanned;
@@ -17,6 +19,8 @@
 
 import org.commonmark.parser.Parser;
 
+import java.util.Collection;
+import java.util.HashSet;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
@@ -48,9 +52,9 @@ public class MentionsPlugin extends AbstractMarkwonPlugin {
     @NonNull
     private final Set<String> noUserCache = ConcurrentHashMap.newKeySet();
     @NonNull
-    private final Context context;
+    private final Collection<ExecutorService> executors = new HashSet<>(2);
     @NonNull
-    private final ExecutorService executor;
+    private final Context context;
     @NonNull
     private final DisplayNameUtil displayNameUtil;
     @NonNull
@@ -61,11 +65,9 @@ public class MentionsPlugin extends AbstractMarkwonPlugin {
     private final AtomicInteger avatarSizeRef = new AtomicInteger();
 
     private MentionsPlugin(@NonNull Context context,
-                           @NonNull ExecutorService executor,
                            @Px int textSize,
                            @ColorInt int color) {
         this.context = context.getApplicationContext();
-        this.executor = executor;
         this.avatarUtil = new AvatarUtil(noUserCache);
         this.displayNameUtil = new DisplayNameUtil(userCache, noUserCache);
         setTextSize(textSize);
@@ -75,7 +77,7 @@ private MentionsPlugin(@NonNull Context context,
     public static MarkwonPlugin create(@NonNull Context context,
                                        @Px int textSize,
                                        @ColorInt int color) {
-        return new MentionsPlugin(context, Executors.newCachedThreadPool(), textSize, color);
+        return new MentionsPlugin(context, textSize, color);
     }
 
     @Override
@@ -103,6 +105,12 @@ public void configureSpansFactory(@NonNull MarkwonSpansFactory.Builder builder)
 
     @Override
     public void beforeSetText(@NonNull TextView textView, @NonNull Spanned markdown) {
+        executors.stream()
+                .filter(not(ExecutorService::isShutdown))
+                .forEach(ExecutorService::shutdownNow);
+
+        executors.removeIf(ExecutorService::isShutdown);
+
         super.beforeSetText(textView, markdown);
     }
 
@@ -111,25 +119,44 @@ public void afterSetText(@NonNull TextView textView) {
         super.afterSetText(textView);
         final var ssoAccount = ssoAccountRef.get();
         if (ssoAccount != null) {
+            final var executor = Executors.newFixedThreadPool(2);
+            executors.add(executor);
             executor.submit(() -> {
                 final var spannable = MarkdownUtil.getContentAsSpannable(textView);
                 try {
                     final var spannableWithDisplayNames = displayNameUtil.insertActualDisplayNames(textView.getContext(), spannable, ssoAccount);
+
+                    if (executor.isShutdown()) return;
+
                     final var spannableWithDisplayNamesAndAvatarPlaceholders = avatarUtil.replacePotentialAvatarsWithPlaceholders(spannableWithDisplayNames);
 
+                    if (executor.isShutdown()) return;
+
                     textView.post(() -> {
                         textView.setText(spannableWithDisplayNamesAndAvatarPlaceholders);
+
+                        if (executor.isShutdown()) return;
+
                         executor.submit(() -> {
                             try {
                                 final var spannableWithDisplayNamesAndActualAvatars = avatarUtil.insertActualAvatars(textView.getContext(), MarkdownUtil.getContentAsSpannable(textView));
-                                Thread.sleep(5_000);
-                                textView.post(() -> textView.setText(spannableWithDisplayNamesAndActualAvatars));
-                            } catch (InterruptedException e) {
-                                e.printStackTrace();
+
+                                if (executor.isShutdown()) return;
+
+                                textView.post(() -> {
+
+                                    if (executor.isShutdown()) return;
+
+                                    textView.setText(spannableWithDisplayNamesAndActualAvatars);
+                                });
+                            } catch (InterruptedException ignored) {
+                            } finally {
+                                executor.shutdown();
                             }
                         });
                     });
-                } catch (InterruptedException | NextcloudFilesAppAccountNotFoundException e) {
+                } catch (InterruptedException ignored) {
+                } catch (NextcloudFilesAppAccountNotFoundException e) {
                     e.printStackTrace();
                 }
             });