Refactor implementation of building hyphenation trees to avoid StackOverflowErrors

The recursive implementation is now rewritten in a sequential way
This solves failures (sometimes flaky ones that depend on JVM config)
of the tests involving hyphenating strings in some of the languages

DEVSIX-2036
DEVSIX-3183

Author: Snipx

layout/src/main/java/com/itextpdf/layout/hyphenation/TernaryTree.java

@@ -15,6 +15,10 @@
  * limitations under the License.
  */
 
+/*
+ * PLEASE NOTE that implementation of "insert" function was refactored to consume less stack memory
+ */
+
 package com.itextpdf.layout.hyphenation;
 
 import java.io.Serializable;
@@ -164,7 +168,7 @@ public void insert(String key, char val) {
         char[] strkey = new char[len--];
         key.getChars(0, len, strkey, 0);
         strkey[len] = 0;
-        root = insert(root, strkey, 0, val);
+        root = insert(new TreeInsertionParams(root, strkey, 0, val));
     }
 
     /**
@@ -178,13 +182,17 @@ public void insert(char[] key, int start, char val) {
         if (freenode + len > eq.length) {
             redimNodeArrays(eq.length + BLOCK_SIZE);
         }
-        root = insert(root, key, start, val);
+        root = insert(new TreeInsertionParams(root, key, start, val));
     }
 
-    /**
-     * The actual insertion function, recursive version.
-     */
-    private char insert(char p, char[] key, int start, char val) {
+    // PLEASE NOTE that this function is a result of refactoring "insert" method which
+    // is a modification of the original work
+    // Returns null if insertion is not needed and the id of the new node if insertion was performed
+    private Character insertNewBranchIfNeeded(TreeInsertionParams params) {
+        char p = params.p;
+        char[] key = params.key;
+        int start = params.start;
+        char val = params.val;
         int len = strlen(key, start);
         if (p == 0) {
             // this means there is no branch, this node will start a new branch.
@@ -204,54 +212,104 @@ private char insert(char p, char[] key, int start, char val) {
                 lo[p] = 0;
             }
             return p;
+        } else {
+            return null;
         }
+    }
 
-        if (sc[p] == 0xFFFF) {
-            // branch is compressed: need to decompress
-            // this will generate garbage in the external key array
-            // but we can do some garbage collection later
-            char pp = freenode++;
-            lo[pp] = lo[p];    // previous pointer to key
-            eq[pp] = eq[p];    // previous pointer to data
-            lo[p] = 0;
-            if (len > 0) {
-                sc[p] = kv.get(lo[pp]);
-                eq[p] = pp;
-                lo[pp]++;
-                if (kv.get(lo[pp]) == 0) {
-                    // key completly decompressed leaving garbage in key array
-                    lo[pp] = 0;
-                    sc[pp] = 0;
-                    hi[pp] = 0;
+    // PLEASE NOTE that this function is a result of refactoring "insert" method which
+    // is a modification of the original work
+    private char insertIntoExistingBranch(TreeInsertionParams params) {
+        char initialP = params.p;
+        TreeInsertionParams paramsToInsertNext = params;
+        while (paramsToInsertNext != null) {
+            char p = paramsToInsertNext.p;
+            // We are inserting into an existing branch hence the id must be non-zero
+            assert p != 0;
+            char[] key = paramsToInsertNext.key;
+            int start = paramsToInsertNext.start;
+            char val = paramsToInsertNext.val;
+            int len = strlen(key, start);
+            paramsToInsertNext = null;
+
+            if (sc[p] == 0xFFFF) {
+                // branch is compressed: need to decompress
+                // this will generate garbage in the external key array
+                // but we can do some garbage collection later
+                char pp = freenode++;
+                lo[pp] = lo[p];    // previous pointer to key
+                eq[pp] = eq[p];    // previous pointer to data
+                lo[p] = 0;
+                if (len > 0) {
+                    sc[p] = kv.get(lo[pp]);
+                    eq[p] = pp;
+                    lo[pp]++;
+                    if (kv.get(lo[pp]) == 0) {
+                        // key completly decompressed leaving garbage in key array
+                        lo[pp] = 0;
+                        sc[pp] = 0;
+                        hi[pp] = 0;
+                    } else {
+                        // we only got first char of key, rest is still there
+                        sc[pp] = 0xFFFF;
+                    }
                 } else {
-                    // we only got first char of key, rest is still there
+                    // In this case we can save a node by swapping the new node
+                    // with the compressed node
                     sc[pp] = 0xFFFF;
+                    hi[p] = pp;
+                    sc[p] = 0;
+                    eq[p] = val;
+                    length++;
+                    break;
                 }
-            } else {
-                // In this case we can save a node by swapping the new node
-                // with the compressed node
-                sc[pp] = 0xFFFF;
-                hi[p] = pp;
-                sc[p] = 0;
-                eq[p] = val;
-                length++;
-                return p;
             }
-        }
-        char s = key[start];
-        if (s < sc[p]) {
-            lo[p] = insert(lo[p], key, start, val);
-        } else if (s == sc[p]) {
-            if (s != 0) {
-                eq[p] = insert(eq[p], key, start + 1, val);
+            char s = key[start];
+            if (s < sc[p]) {
+                TreeInsertionParams branchParams = new TreeInsertionParams(lo[p], key, start, val);
+                Character insertNew = insertNewBranchIfNeeded(branchParams);
+                if (insertNew == null) {
+                    paramsToInsertNext = branchParams;
+                } else {
+                    lo[p] = insertNew;
+                }
+            } else if (s == sc[p]) {
+                if (s != 0) {
+                    TreeInsertionParams branchParams = new TreeInsertionParams(eq[p], key, start + 1, val);
+                    Character insertNew = insertNewBranchIfNeeded(branchParams);
+                    if (insertNew == null) {
+                        paramsToInsertNext = branchParams;
+                    } else {
+                        eq[p] = insertNew;
+                    }
+                } else {
+                    // key already in tree, overwrite data
+                    eq[p] = val;
+                }
             } else {
-                // key already in tree, overwrite data
-                eq[p] = val;
+                TreeInsertionParams branchParams = new TreeInsertionParams(hi[p], key, start, val);
+                Character insertNew = insertNewBranchIfNeeded(branchParams);
+                if (insertNew == null) {
+                    paramsToInsertNext = branchParams;
+                } else {
+                    hi[p] = insertNew;
+                }
             }
+        }
+        return initialP;
+    }
+
+    /**
+     * The actual insertion function, recursive version.
+     * PLEASE NOTE that the implementation has been adapted to consume less stack memory
+     */
+    private char insert(TreeInsertionParams params) {
+        Character newBranch = insertNewBranchIfNeeded(params);
+        if (newBranch == null) {
+            return insertIntoExistingBranch(params);
         } else {
-            hi[p] = insert(hi[p], key, start, val);
+            return (char)newBranch;
         }
-        return p;
     }
 
     /**
@@ -514,5 +572,21 @@ private void compact(CharVector kx, TernaryTree map, char p) {
     public Enumeration keys() {
         return new TernaryTreeIterator(this);
     }
+
+    // PLEASE NOTE that this is a helper class that was added as a result of the file modification
+    // and is not a part of the original file
+    private static class TreeInsertionParams {
+        char p;
+        char[] key;
+        int start;
+        char val;
+
+        public TreeInsertionParams(char p, char[] key, int start, char val) {
+            this.p = p;
+            this.key = key;
+            this.start = start;
+            this.val = val;
+        }
+    }
 }
 

layout/src/test/java/com/itextpdf/layout/HyphenateResultTest.java

@@ -47,22 +47,19 @@ This file is part of the iText (R) project.
 import com.itextpdf.test.ExtendedITextTest;
 import com.itextpdf.test.annotations.type.IntegrationTest;
 import org.junit.Assert;
-import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
 @Category(IntegrationTest.class)
 public class HyphenateResultTest extends ExtendedITextTest {
 
     @Test
-    @Ignore("DEVSIX-2036")
     public void ukraineHyphenTest() {
         //здравствуйте
         testHyphenateResult("uk", "\u0437\u0434\u0440\u0430\u0432\u0441\u0442\u0432\u0443\u0439", new int[]{5});
     }
 
     @Test
-    @Ignore("DEVSIX-2036")
     public void ukraineNoneHyphenTest() {
         //день
         testHyphenateResult("uk", "\u0434\u0435\u043D\u044C", null);

layout/src/test/java/com/itextpdf/layout/HyphenateTest.java

@@ -48,7 +48,6 @@ This file is part of the iText (R) project.
 import com.itextpdf.test.ExtendedITextTest;
 import com.itextpdf.test.annotations.type.IntegrationTest;
 import org.junit.Assert;
-import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
@@ -135,7 +134,6 @@ public class HyphenateTest extends ExtendedITextTest {
     private List<String> errors = new ArrayList<>();
 
     @Test
-    @Ignore("DEVSIX-2036")
     public void runTest() {
         for (TestParams param : params) {
             tryHyphenate(param.lang, param.testWorld, param.shouldPass);