Fix a bug with the string pool.

This also adds some optional debug code to detect more easily possible
future occurrences of the same problem.

Bug: 5195017
Change-Id: I2558b468e46f7090de868f1ec2dc9e24895d670f

2 files changed, 23 insertions, 2 deletions

diff --git a/java/src/com/android/inputmethod/latin/StringBuilderPool.java b/java/src/com/android/inputmethod/latin/StringBuilderPool.java
index 66f123731..a663ed43e 100644
--- a/java/src/com/android/inputmethod/latin/StringBuilderPool.java
+++ b/java/src/com/android/inputmethod/latin/StringBuilderPool.java
@@ -26,12 +26,20 @@ import java.util.List;
 public class StringBuilderPool {
     // Singleton
     private static final StringBuilderPool sInstance = new StringBuilderPool();
+    private static final boolean DEBUG = false;
     private StringBuilderPool() {}
-    // TODO: Make this a normal array with a size of 20
+    // TODO: Make this a normal array with a size of 20, or a ConcurrentQueue
     private final List<StringBuilder> mPool =
             Collections.synchronizedList(new ArrayList<StringBuilder>());
 
     public static StringBuilder getStringBuilder(final int initialSize) {
+        // TODO: although the pool is synchronized, the following is not thread-safe.
+        // Two threads entering this at the same time could take the same size of the pool and the
+        // second to attempt removing this index from the pool would crash with an
+        // IndexOutOfBoundsException.
+        // At the moment this pool is only used in Suggest.java and only in one thread so it's
+        // okay. The simplest thing to do here is probably to replace the ArrayList with a
+        // ConcurrentQueue.
         final int poolSize = sInstance.mPool.size();
         final StringBuilder sb = poolSize > 0 ? (StringBuilder) sInstance.mPool.remove(poolSize - 1)
                 : new StringBuilder(initialSize);
@@ -40,6 +48,12 @@ public class StringBuilderPool {
     }
 
     public static void recycle(final StringBuilder garbage) {
+        if (DEBUG) {
+            final int gid = garbage.hashCode();
+            for (final StringBuilder q : sInstance.mPool) {
+                if (gid == q.hashCode()) throw new RuntimeException("Duplicate id " + gid);
+            }
+        }
         sInstance.mPool.add(garbage);
     }
 
diff --git a/java/src/com/android/inputmethod/latin/Suggest.java b/java/src/com/android/inputmethod/latin/Suggest.java
index e3cb6987a..29b629576 100644
--- a/java/src/com/android/inputmethod/latin/Suggest.java
+++ b/java/src/com/android/inputmethod/latin/Suggest.java
@@ -284,7 +284,14 @@ public class Suggest implements Dictionary.WordCallback {
     }
 
     protected void addBigramToSuggestions(CharSequence bigram) {
-        mSuggestions.add(bigram);
+        // TODO: Try to be a little more shrewd with resource allocation.
+        // At the moment we copy this object because the StringBuilders are pooled (see
+        // StringBuilderPool.java) and when we are finished using mSuggestions and
+        // mBigramSuggestions we will take everything from both and insert them back in the
+        // pool, so we can't allow the same object to be in both lists at the same time.
+        final StringBuilder sb = StringBuilderPool.getStringBuilder(getApproxMaxWordLength());
+        sb.append(bigram);
+        mSuggestions.add(sb);
     }
 
     // TODO: cleanup dictionaries looking up and suggestions building with SuggestedWords.Builder