[intl] Store the collator as a Managed

The lifetime of the collator is handled by the JavaScript heap. At the
moment this is implemented with a weak GlobalHandle. With this CL I
change the implementation to use a Managed object instead. In addition I
did some code cleanup.

The main reason for using a Managed is an lsan problem. The final GC in
d8 is triggered before all pending WebAssembly compilations get
canceled. Via the native context, WebAssembly compilation can keep the
Collator wrapper alive, and therefore the collator is never deallocated.
Managed, however, get processed at isolate teardown, independent of the
reachability of the Managed.

TEST=mjsunit/regress/regress-813440

Bug: chromium:813440
Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng
Change-Id: Ie727eb1aff2144586eb36426cc44a32357c0f822
Reviewed-on: https://chromium-review.googlesource.com/956069
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51886}

src/objects/intl-objects.cc

@@ -14,6 +14,7 @@
 #include "src/factory.h"
 #include "src/global-handles.h"
 #include "src/isolate.h"
+#include "src/managed.h"
 #include "src/objects-inl.h"
 #include "src/property-descriptor.h"
 #include "unicode/brkiter.h"
@@ -901,10 +902,11 @@ void NumberFormat::DeleteNumberFormat(const v8::WeakCallbackInfo<void>& data) {
   GlobalHandles::Destroy(reinterpret_cast<Object**>(data.GetParameter()));
 }
 
-icu::Collator* Collator::InitializeCollator(Isolate* isolate,
-                                            Handle<String> locale,
-                                            Handle<JSObject> options,
-                                            Handle<JSObject> resolved) {
+bool Collator::InitializeCollator(Isolate* isolate,
+                                  Handle<JSObject> collator_holder,
+                                  Handle<String> locale,
+                                  Handle<JSObject> options,
+                                  Handle<JSObject> resolved) {
   v8::Isolate* v8_isolate = reinterpret_cast<v8::Isolate*>(isolate);
   // Convert BCP47 into ICU locale format.
   UErrorCode status = U_ZERO_ERROR;
@@ -916,7 +918,7 @@ icu::Collator* Collator::InitializeCollator(Isolate* isolate,
     uloc_forLanguageTag(*bcp47_locale, icu_result, ULOC_FULLNAME_CAPACITY,
                         &icu_length, &status);
     if (U_FAILURE(status) || icu_length == 0) {
-      return nullptr;
+      return false;
     }
     icu_locale = icu::Locale(icu_result);
   }
@@ -938,17 +940,16 @@ icu::Collator* Collator::InitializeCollator(Isolate* isolate,
     SetResolvedCollatorSettings(isolate, icu_locale, collator, resolved);
   }
 
-  return collator;
+  Handle<Managed<icu::Collator>> managed =
+      Managed<icu::Collator>::From(isolate, collator);
+  collator_holder->SetEmbedderField(0, *managed);
+
+  return true;
 }
 
 icu::Collator* Collator::UnpackCollator(Isolate* isolate,
                                         Handle<JSObject> obj) {
-  return reinterpret_cast<icu::Collator*>(obj->GetEmbedderField(0));
-}
-
-void Collator::DeleteCollator(const v8::WeakCallbackInfo<void>& data) {
-  delete reinterpret_cast<icu::Collator*>(data.GetInternalField(0));
-  GlobalHandles::Destroy(reinterpret_cast<Object**>(data.GetParameter()));
+  return Managed<icu::Collator>::cast(obj->GetEmbedderField(0))->get();
 }
 
 bool PluralRules::InitializePluralRules(Isolate* isolate, Handle<String> locale,

src/objects/intl-objects.h

@@ -77,20 +77,17 @@ class NumberFormat {
 
 class Collator {
  public:
-  // Create a collator for the specificied locale and options. Returns the
-  // resolved settings for the locale / options.
-  static icu::Collator* InitializeCollator(Isolate* isolate,
-                                           Handle<String> locale,
-                                           Handle<JSObject> options,
-                                           Handle<JSObject> resolved);
+  // Create a collator for the specificied locale and options. Stores the
+  // collator in the provided collator_holder.
+  static bool InitializeCollator(Isolate* isolate,
+                                 Handle<JSObject> collator_holder,
+                                 Handle<String> locale,
+                                 Handle<JSObject> options,
+                                 Handle<JSObject> resolved);
 
   // Unpacks collator object from corresponding JavaScript object.
   static icu::Collator* UnpackCollator(Isolate* isolate, Handle<JSObject> obj);
 
-  // Release memory we allocated for the Collator once the JS object that holds
-  // the pointer gets garbage collected.
-  static void DeleteCollator(const v8::WeakCallbackInfo<void>& data);
-
   // Layout description.
   static const int kCollator = JSObject::kHeaderSize;
   static const int kSize = kCollator + kPointerSize;

src/runtime/runtime-intl.cc

@@ -501,23 +501,16 @@ RUNTIME_FUNCTION(Runtime_CreateCollator) {
   Handle<JSFunction> constructor(
       isolate->native_context()->intl_collator_function());
 
-  Handle<JSObject> local_object;
-  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, local_object,
+  Handle<JSObject> collator_holder;
+  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, collator_holder,
                                      JSObject::New(constructor, constructor));
 
-  // Set collator as embedder field of the resulting JS object.
-  icu::Collator* collator =
-      Collator::InitializeCollator(isolate, locale, options, resolved);
-
-  if (!collator) return isolate->ThrowIllegalOperation();
-
-  local_object->SetEmbedderField(0, reinterpret_cast<Smi*>(collator));
+  if (!Collator::InitializeCollator(isolate, collator_holder, locale, options,
+                                    resolved)) {
+    return isolate->ThrowIllegalOperation();
+  }
 
-  Handle<Object> wrapper = isolate->global_handles()->Create(*local_object);
-  GlobalHandles::MakeWeak(wrapper.location(), wrapper.location(),
-                          Collator::DeleteCollator,
-                          WeakCallbackType::kInternalFields);
-  return *local_object;
+  return *collator_holder;
 }
 
 RUNTIME_FUNCTION(Runtime_InternalCompare) {

test/mjsunit/regress/regress-813440.js

+// Copyright 2018 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --invoke-weak-callbacks --omit-quit --wasm-async-compilation --expose-wasm --allow-natives-syntax
+
+load("test/mjsunit/wasm/wasm-constants.js");
+load("test/mjsunit/wasm/wasm-module-builder.js");
+
+const builder = new WasmModuleBuilder();
+builder.addFunction('f', kSig_i_v).addBody([kExprI32Const, 42]);
+const buffer = builder.toBuffer();
+// Start async compilation, but don't wait for it to finish.
+const module = WebAssembly.compile(buffer);
+
+// This create the collator.
+'퓛'.localeCompare();