ICU-20941 Fix ResourceTable lifetime to make ResourceTracer happy

This is a separate commit from the previous ICU-20941 commit for the
sake of documentation / future code archaeology.

See #1588

diff --git a/icu4c/source/common/resource.h b/icu4c/source/common/resource.h
index 3795694412a058d5ef9c525b11f687fb4d5f5d9f..48f5b9fa6ec7cc55274f3699d146609d642d95be 100644 (file)
--- a/icu4c/source/common/resource.h
+++ b/icu4c/source/common/resource.h
@@ -274,8 +274,10 @@ public:
      *
      * @param key The key string of the enumeration-start resource.
      *     Empty if the enumeration starts at the top level of the bundle.
-     * @param value Call getArray() or getTable() as appropriate.
-     *     Then reuse for output values from Array and Table getters.
+     * @param value Call getArray() or getTable() as appropriate. Then reuse for
+     *     output values from Array and Table getters. Note: ResourceTable and
+     *     ResourceArray instances must outlive the ResourceValue instance for
+     *     ResourceTracer to be happy.
      * @param noFallback true if the bundle has no parent;
      *     that is, its top-level table has the nofallback attribute,
      *     or it is the root bundle of a locale tree.

diff --git a/icu4c/source/common/restrace.cpp b/icu4c/source/common/restrace.cpp
index 5c6498850e2f8f5e58c3d13dc0d97db212451851..1f83372d682a7f58282b096d25e1c1a3ea67f3a8 100644 (file)
--- a/icu4c/source/common/restrace.cpp
+++ b/icu4c/source/common/restrace.cpp
@@ -54,6 +54,9 @@ void ResourceTracer::traceOpen() const {
 
 CharString& ResourceTracer::getFilePath(CharString& output, UErrorCode& status) const {
     if (fResB) {
+        // Note: if you get a segfault around here, check that ResourceTable and
+        // ResourceArray instances outlive ResourceValue instances referring to
+        // their contents:
         output.append(fResB->fData->fPath, status);
         output.append('/', status);
         output.append(fResB->fData->fName, status);

diff --git a/icu4c/source/i18n/number_longnames.cpp b/icu4c/source/i18n/number_longnames.cpp
index 41d0e7c3d750be3013d0f5cc9e58648da517161e..0ec75ea0cc88380ec32495ad11ee0a27dc47a606 100644 (file)
--- a/icu4c/source/i18n/number_longnames.cpp
+++ b/icu4c/source/i18n/number_longnames.cpp
@@ -265,7 +265,8 @@ class InflectedPluralSink : public ResourceSink {
                 continue;
             }
             ResourceTable genderTable = value.getTable(status);
-            if (loadForPluralForm(genderTable, value, status)) {
+            ResourceTable caseTable; // This instance has to outlive `value`
+            if (loadForPluralForm(genderTable, caseTable, value, status)) {
                 outArray[pluralIndex] = value.getUnicodeString(status);
             }
         }
@@ -275,17 +276,22 @@ class InflectedPluralSink : public ResourceSink {
     // Tries to load data for the configured gender from `genderTable`. Returns
     // true if found, returning the data in `value`. The returned data will be
     // for the configured gender if found, falling back to "neuter" and
-    // no-gender if not.
-    bool loadForPluralForm(const ResourceTable &genderTable, ResourceValue &value, UErrorCode &status) {
+    // no-gender if not. The caseTable parameter holds the intermediate
+    // ResourceTable for the sake of lifetime management.
+    bool loadForPluralForm(const ResourceTable &genderTable,
+                           ResourceTable &caseTable,
+                           ResourceValue &value,
+                           UErrorCode &status) {
         if (uprv_strcmp(gender, "") != 0) {
-            if (loadForGender(genderTable, gender, value, status)) {
+            if (loadForGender(genderTable, gender, caseTable, value, status)) {
                 return true;
             }
-            if (uprv_strcmp(gender, "neuter") != 0 && loadForGender(genderTable, "neuter", value, status)) {
+            if (uprv_strcmp(gender, "neuter") != 0 &&
+                loadForGender(genderTable, "neuter", caseTable, value, status)) {
                 return true;
             }
         }
-        if (loadForGender(genderTable, "_", value, status)) {
+        if (loadForGender(genderTable, "_", caseTable, value, status)) {
             return true;
         }
         return false;
@@ -296,13 +302,14 @@ class InflectedPluralSink : public ResourceSink {
     // the configured case if found, falling back to "nominative" and no-case if
     // not.
     bool loadForGender(const ResourceTable &genderTable,
-                   const char *genderVal,
-                   ResourceValue &value,
-                   UErrorCode &status) {
+                       const char *genderVal,
+                       ResourceTable &caseTable,
+                       ResourceValue &value,
+                       UErrorCode &status) {
         if (!genderTable.findValue(genderVal, value)) {
             return false;
         }
-        ResourceTable caseTable = value.getTable(status);
+        caseTable = value.getTable(status);
         if (uprv_strcmp(caseVariant, "") != 0) {
             if (loadForCase(caseTable, caseVariant, value)) {
                 return true;
@@ -320,9 +327,7 @@ class InflectedPluralSink : public ResourceSink {
 
     // Tries to load data for the given case from `caseTable`. Returns true if
     // found, returning the data in `value`.
-    bool loadForCase(const ResourceTable &caseTable,
-                 const char *caseValue,
-                 ResourceValue &value) {
+    bool loadForCase(const ResourceTable &caseTable, const char *caseValue, ResourceValue &value) {
         if (!caseTable.findValue(caseValue, value)) {
             return false;
         }