[[ Bug 22011 ]] Fix memory leak when using LCB's char iterator

runrevmark · runrevmark · commit bd91ba39255f · 2019-04-18T20:33:52.000+01:00
This patch fixes a memory leak which occurs when using LCB's repeat
for each char control structure when the loop terminates early, e.g.
due to using exit repeat or return within the loop.

The reason the leak occurs is that, previously, the underlying iterator
required a memory allocation at the start, which required deallocation
at the end. However, LCB cannot support such iterator requirements
correctly at the moment.

To fix the issue, the char iterator implementation has been changed to
use the underlying grapheme iterator directly - this does not require
any memory allocation to hold state as it works by looking at adjacent
codepoints, and thus can be implemented in the same fashion as the other
iterators (i.e. by storing the current codeunit offset in the iterator
state pointer).
diff --git a/docs/notes/bugfix-22011.md b/docs/notes/bugfix-22011.md
@@ -0,0 +1 @@
+# Fix memory leak when using repeat for each in LCB in early termination cases
diff --git a/libscript/src/module-char.cpp b/libscript/src/module-char.cpp
@@ -246,28 +246,29 @@ extern "C" MC_DLLEXPORT_DEF void MCCharExecDeleteLastCharOf(MCStringRef& x_targe
 //   end repeat
 // Will result in tChar containing the value it had at the point of end repeat.
 extern "C" MC_DLLEXPORT_DEF bool MCCharRepeatForEachChar(void*& x_iterator, MCStringRef& r_iterand, MCStringRef p_string)
-{
-    MCTextChunkIterator *t_iterator;
-    bool t_first;
-    t_first = false;
+{ 
+    uindex_t t_offset;
+    t_offset = (uindex_t)(uintptr_t)x_iterator;
     
-    if ((uintptr_t)x_iterator == 0)
-    {
-        t_first = true;
-        t_iterator = MCChunkCreateTextChunkIterator(p_string, nil, kMCChunkTypeCharacter, nil, nil, kMCStringOptionCompareExact);
-    }
-    else
-        t_iterator = (MCTextChunkIterator *)x_iterator;
+    uindex_t t_length =
+            MCStringGetLength(p_string);
+    if (t_offset == t_length)
+        return false;
+
+    uindex_t t_next =
+            MCStringGraphemeBreakIteratorAdvance(p_string,
+                                                 t_offset);
+    if (t_next == (uindex_t)-1)
+        t_next = t_length;
     
-    if (t_iterator -> Next())
-        t_iterator -> CopyString(r_iterand);
-    else
+    if (!MCStringCopySubstring(p_string,
+                               MCRangeMake(t_offset, t_next - t_offset),
+                               r_iterand))
     {
-        delete t_iterator;
         return false;
     }
     
-    x_iterator = (void *)(t_iterator);
+    x_iterator = (void *)(uintptr_t)t_next;
     
     return true;
 }
diff --git a/tests/lcb/stdlib/char.lcb b/tests/lcb/stdlib/char.lcb
@@ -397,13 +397,26 @@ public handler TestRepeatChar()
 	variable tCount
 
 	put 0 into tCount
-
 	repeat for each char tIter in "xyz"
 		add 1 to tCount
 		test "repeatchar (iter)" when tIter is char tCount of "xyz"
 	end repeat
-
 	test "repeatchar (count)" when tCount is 3
+
+	put 0 into tCount
+	repeat for each char tIter in "a\u{1d11e}c"
+		add 1 to tCount
+		test "repeatchar SMP (iter)" when tIter is char tCount of "a\u{1d11e}c"
+	end repeat
+	test "repeatchar SMP (count)" when tCount is 3
+
+	put 0 into tCount
+	repeat for each char tIter in "av\u{fe0e}c"
+		add 1 to tCount
+		test "repeatchar VAR (iter)" when tIter is char tCount of "av\u{fe0e}c"
+	end repeat
+	test "repeatchar VAR (count)" when tCount is 3
+
 end handler
 
 handler TestReverse_Inplace(in pDesc as String, \

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+# Fix memory leak when using repeat for each in LCB in early termination cases`