From 6d276698b6102918b6ac1d968dd7ec9a151bae52 Mon Sep 17 00:00:00 2001
From: Peter Nelson <peter1138@openttd.org>
Date: Sat, 27 Jan 2024 20:13:42 +0000
Subject: [PATCH] Fix: Memory leak in ICUParagraphLayout::NextLine() (#11895)

This function calls icu::BreakIterator::createLineInstance() but does not clean up after it.

Instead use a static instance that is cloned (for thread-safety) and deleted as necessary.
---
 src/gfx_layout.cpp     | 10 ++++++++++
 src/gfx_layout.h       |  1 +
 src/gfx_layout_icu.cpp | 29 +++++++++++++++++++++++++----
 src/gfx_layout_icu.h   |  6 ++++++
 src/strings.cpp        |  3 +++
 5 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/src/gfx_layout.cpp b/src/gfx_layout.cpp
index 6909b4cd28..91280e5555 100644
--- a/src/gfx_layout.cpp
+++ b/src/gfx_layout.cpp
@@ -334,6 +334,16 @@ Font *Layouter::GetFont(FontSize size, TextColour colour)
 	return fonts[size][colour].get();
 }
 
+/**
+ * Perform initialization of layout engine.
+ */
+void Layouter::Initialize()
+{
+#if defined(WITH_ICU_I18N) && defined(WITH_HARFBUZZ)
+	ICUParagraphLayoutFactory::InitializeLayouter();
+#endif /* WITH_ICU_I18N && WITH_HARFBUZZ */
+}
+
 /**
  * Reset cached font information.
  * @param size Font size to reset.
diff --git a/src/gfx_layout.h b/src/gfx_layout.h
index fc0340b179..636b5cdadc 100644
--- a/src/gfx_layout.h
+++ b/src/gfx_layout.h
@@ -179,6 +179,7 @@ public:
 	Point GetCharPosition(std::string_view::const_iterator ch) const;
 	ptrdiff_t GetCharAtPosition(int x, size_t line_index) const;
 
+	static void Initialize();
 	static void ResetFontCache(FontSize size);
 	static void ResetLineCache();
 	static void ReduceLineCache();
diff --git a/src/gfx_layout_icu.cpp b/src/gfx_layout_icu.cpp
index d93a9eb771..e0008f2fa5 100644
--- a/src/gfx_layout_icu.cpp
+++ b/src/gfx_layout_icu.cpp
@@ -381,6 +381,30 @@ std::vector<ICURun> ItemizeStyle(std::vector<ICURun> &runs_current, FontMap &fon
 	return new ICUParagraphLayout(runs, buff, length);
 }
 
+/* static */ std::unique_ptr<icu::BreakIterator> ICUParagraphLayoutFactory::break_iterator;
+
+/**
+ * Initialize data needed for the ICU layouter.
+ */
+/* static */ void ICUParagraphLayoutFactory::InitializeLayouter()
+{
+	auto locale = icu::Locale(_current_language->isocode);
+	UErrorCode status = U_ZERO_ERROR;
+	ICUParagraphLayoutFactory::break_iterator.reset(icu::BreakIterator::createLineInstance(locale, status));
+	assert(U_SUCCESS(status));
+}
+
+/**
+ * Get a thread-safe line break iterator.
+ * @returns unique_ptr managed BreakIterator instance.
+ */
+/* static */ std::unique_ptr<icu::BreakIterator> ICUParagraphLayoutFactory::GetBreakIterator()
+{
+	assert(ICUParagraphLayoutFactory::break_iterator != nullptr);
+
+	return std::unique_ptr<icu::BreakIterator>(ICUParagraphLayoutFactory::break_iterator->clone());
+}
+
 std::unique_ptr<const ICUParagraphLayout::Line> ICUParagraphLayout::NextLine(int max_width)
 {
 	std::vector<ICURun>::iterator start_run = this->current_run;
@@ -413,11 +437,8 @@ std::unique_ptr<const ICUParagraphLayout::Line> ICUParagraphLayout::NextLine(int
 	/* If the text does not fit into the available width, find a suitable breaking point. */
 	int new_partial_length = 0;
 	if (cur_width > max_width) {
-		auto locale = icu::Locale(_current_language->isocode);
-
 		/* Create a break-iterator to find a good place to break lines. */
-		UErrorCode err = U_ZERO_ERROR;
-		auto break_iterator = icu::BreakIterator::createLineInstance(locale, err);
+		auto break_iterator = ICUParagraphLayoutFactory::GetBreakIterator();
 		break_iterator->setText(icu::UnicodeString(this->buff, this->buff_length));
 
 		auto overflow_run = last_run - 1;
diff --git a/src/gfx_layout_icu.h b/src/gfx_layout_icu.h
index 07f2f68721..f0adaeb15b 100644
--- a/src/gfx_layout_icu.h
+++ b/src/gfx_layout_icu.h
@@ -12,6 +12,7 @@
 
 #include "gfx_layout.h"
 
+#include <unicode/brkiter.h>
 #include <unicode/ustring.h>
 
 /**
@@ -26,6 +27,11 @@ public:
 
 	static ParagraphLayouter *GetParagraphLayout(UChar *buff, UChar *buff_end, FontMap &fontMapping);
 	static size_t AppendToBuffer(UChar *buff, const UChar *buffer_last, char32_t c);
+
+	static void InitializeLayouter();
+	static std::unique_ptr<icu::BreakIterator> GetBreakIterator();
+private:
+	static std::unique_ptr<icu::BreakIterator> break_iterator;
 };
 
 #endif /* GFX_LAYOUT_ICU_H */
diff --git a/src/strings.cpp b/src/strings.cpp
index bbf02aaf83..a70ddf57f4 100644
--- a/src/strings.cpp
+++ b/src/strings.cpp
@@ -37,6 +37,7 @@
 #include "network/network_content_gui.h"
 #include "newgrf_engine.h"
 #include "core/backup_type.hpp"
+#include "gfx_layout.h"
 #include <stack>
 #include <charconv>
 
@@ -1976,6 +1977,8 @@ bool ReadLanguagePack(const LanguageMetadata *lang)
 	}
 #endif /* WITH_ICU_I18N */
 
+	Layouter::Initialize();
+
 	/* Some lists need to be sorted again after a language change. */
 	ReconsiderGameScriptLanguage();
 	InitializeSortedCargoSpecs();