Browse Source

Defer ICU initialization until the Intl native interfaces are initialized

This modifies a recent change that attempted to initialize ICU before calling any Intl functions in an effort to catch OOM scenarios early. I originally added the code to IntlEngineInterfaceExtensionObject::Initialize, which seemed reasonable, but I forgot that Initialize is always called as part of JavascriptLibrary initialization. As a result, this code was being run on startup at all times. This broke Node-ChakraCore because they have special handling for ICU data, and we were calling u_init before Node's data initialization logic ran. The fix is to initialize ICU in the deferred type handler initialization for the Intl native interfaces, which should only run when script code actually uses Intl (which will always be after Node sets up ICU themselves).
Jack Horton (CHAKRA) 8 năm trước cách đây
mục cha
commit
94b2fddfbc
1 tập tin đã thay đổi với 16 bổ sung18 xóa
  1. 16 18
      lib/Runtime/Library/IntlEngineInterfaceExtensionObject.cpp

+ 16 - 18
lib/Runtime/Library/IntlEngineInterfaceExtensionObject.cpp

@@ -469,24 +469,6 @@ namespace Js
             {
                 library->AddMember(library->GetIntlObject(), PropertyIds::platform, this->intlNativeInterfaces);
             }
-
-#ifdef INTL_ICU
-            // when using ICU, we need to call u_init to ensure that ICU is functioning properly before allowing Intl to continue.
-            // u_init will cause the data file to be loaded, and if we don't have enough memory to do so, we can throw OutOfMemory here.
-            // This is to protect against spurious U_MISSING_RESOURCE_ERRORs and U_FILE_ACCESS_ERRORs coming from early-lifecycle
-            // functions that require ICU data.
-            // See OS#16897150, OS#16896933, and others relating to bad statuses returned by GetLocaleData and IsLocaleAvailable
-            UErrorCode status = U_ZERO_ERROR;
-            u_init(&status);
-            if (status == U_MEMORY_ALLOCATION_ERROR || status == U_FILE_ACCESS_ERROR || status == U_MISSING_RESOURCE_ERROR)
-            {
-                // Trace that this happens in case there are build system changes that actually cause the data file to be not found
-                INTL_TRACE("Could not initialize ICU - u_init returned status %S", u_errorName(status));
-                Throw::OutOfMemory();
-            }
-
-            AssertOrFailFastMsg(U_SUCCESS(status), "u_init returned non-OOM failure");
-#endif
         }
         wasInitialized = true;
     }
@@ -520,6 +502,22 @@ namespace Js
         library->AddMember(intlNativeInterfaces, Js::PropertyIds::winglob, library->GetTrue());
 #else
         library->AddMember(intlNativeInterfaces, Js::PropertyIds::winglob, library->GetFalse());
+
+        // when using ICU, we need to call u_init to ensure that ICU is functioning properly before allowing Intl to continue.
+        // u_init will cause the data file to be loaded, and if we don't have enough memory to do so, we can throw OutOfMemory here.
+        // This is to protect against spurious U_MISSING_RESOURCE_ERRORs and U_FILE_ACCESS_ERRORs coming from early-lifecycle
+        // functions that require ICU data.
+        // See OS#16897150, OS#16896933, and others relating to bad statuses returned by GetLocaleData and IsLocaleAvailable
+        UErrorCode status = U_ZERO_ERROR;
+        u_init(&status);
+        if (status == U_MEMORY_ALLOCATION_ERROR || status == U_FILE_ACCESS_ERROR || status == U_MISSING_RESOURCE_ERROR)
+        {
+            // Trace that this happens in case there are build system changes that actually cause the data file to be not found
+            INTL_TRACE("Could not initialize ICU - u_init returned status %S", u_errorName(status));
+            Throw::OutOfMemory();
+        }
+
+        AssertOrFailFastMsg(U_SUCCESS(status), "u_init returned non-OOM failure");
 #endif
 
         intlNativeInterfaces->SetHasNoEnumerableProperties(true);