feat: scope worker-created extended class names to their isolate

NativeScript/runtime/ClassBuilder.cpp

@@ -49,15 +49,17 @@ constexpr int kMaxConsecutiveAllocFailures = 100;
 
 // Moved this method in a separate .cpp file because ARC destroys the class
 // created with objc_allocateClassPair when the control leaves this method scope
-Class ClassBuilder::GetExtendedClass(std::string baseClassName,
-                                     std::string staticClassName,
-                                     std::string suffix) {
+Class ClassBuilder::GetExtendedClass(const std::string& baseClassName,
+                                     const std::string& staticClassName,
+                                     int isolateId) {
   Class baseClass = objc_getClass(baseClassName.c_str());
-  std::string name =
-      !staticClassName.empty()
-          ? staticClassName
-          : baseClassName + suffix + "_" +
-                std::to_string(++ClassBuilder::classNameCounter_);
+  std::string name = staticClassName;
+  if (name.empty()) {
+    name = baseClassName;
+    name += std::to_string(isolateId);
+    name += "__";
+    name += std::to_string(++ClassBuilder::classNameCounter_);
+  }
   // Allocation failure is the collision signal (objc_getClass beforehand
   // would race), but that only detects *registered* names — hence the lock
   // spanning allocate -> register.

NativeScript/runtime/ClassBuilder.h

@@ -27,9 +27,9 @@ class ClassBuilder {
  public:
   static v8::Local<v8::FunctionTemplate> GetExtendFunction(
       v8::Isolate* isolate, const InterfaceMeta* interfaceMeta);
-  static Class GetExtendedClass(std::string baseClassName,
-                                std::string staticClassName,
-                                std::string suffix);
+  static Class GetExtendedClass(const std::string& baseClassName,
+                                const std::string& staticClassName,
+                                int isolateId);
 
   static void RegisterBaseTypeScriptExtendsFunction(
       v8::Local<v8::Context> context);

NativeScript/runtime/ClassBuilder.mm

@@ -17,6 +17,20 @@
 
 namespace tns {
 
+namespace {
+// Worker-created named classes must not claim process-global objc names:
+// verbatim names are the main isolate's contract (storyboards,
+// NSClassFromString and other name-based native lookups), and a worker
+// winning the registration race for one would nondeterministically demote the
+// main isolate's class to a collision suffix.
+void ScopeClassNameToIsolate(std::string& name, int isolateId) {
+  if (!name.empty() && Runtime::IsWorker()) {
+    name += '_';
+    name += std::to_string(isolateId);
+  }
+}
+}  // namespace
+
 Local<FunctionTemplate> ClassBuilder::GetExtendFunction(Isolate* isolate,
                                                         const InterfaceMeta* interfaceMeta) {
   CacheItem* item = new CacheItem(interfaceMeta, nullptr);
@@ -62,8 +76,8 @@
     auto cache = Caches::Get(isolate);
     auto isolateId = cache->getIsolateId();
 
-    Class extendedClass = ClassBuilder::GetExtendedClass(baseClassName, staticClassName,
-                                                         std::to_string(isolateId) + "_");
+    ScopeClassNameToIsolate(staticClassName, isolateId);
+    Class extendedClass = ClassBuilder::GetExtendedClass(baseClassName, staticClassName, isolateId);
     tns::Assert(extendedClass != nil, isolate);
     class_addProtocol(extendedClass, @protocol(TNSDerivedClass));
     class_addProtocol(object_getClass(extendedClass), @protocol(TNSDerivedClass));
@@ -205,8 +219,9 @@
         std::string extendedClassName = tns::ToString(isolate, extendedClassCtorFunc->GetName());
 
         auto isolateId = cache->getIsolateId();
-        __block Class extendedClass = ClassBuilder::GetExtendedClass(
-            baseClassName, extendedClassName, std::to_string(isolateId) + "_");
+        ScopeClassNameToIsolate(extendedClassName, isolateId);
+        __block Class extendedClass =
+            ClassBuilder::GetExtendedClass(baseClassName, extendedClassName, isolateId);
         tns::Assert(extendedClass != nil, isolate);
         class_addProtocol(extendedClass, @protocol(TNSDerivedClass));
         class_addProtocol(object_getClass(extendedClass), @protocol(TNSDerivedClass));

TestRunner/app/tests/ExtendedClassNamingTests.js

@@ -0,0 +1,40 @@
+describe("Extended class naming", function () {
+    it("keeps explicit main-isolate class names verbatim", function () {
+        var MainClaim = NSObject.extend({}, { name: "TNSMainVerbatimName" });
+        expect(NSStringFromClass(MainClaim)).toBe("TNSMainVerbatimName");
+    });
+
+    it("scopes worker-created explicit class names to their isolate", function (done) {
+        var worker = new Worker("~/shared/Workers/EvalWorker.js");
+        worker.onmessage = function (msg) {
+            worker.terminate();
+            var workerName = msg.data.name;
+            expect(workerName).not.toBe("TNSWorkerNameClaim");
+            expect(workerName.indexOf("TNSWorkerNameClaim")).toBe(0);
+            // The verbatim name must remain available to the main isolate.
+            var MainClass = NSObject.extend({}, { name: "TNSWorkerNameClaim" });
+            expect(NSStringFromClass(MainClass)).toBe("TNSWorkerNameClaim");
+            done();
+        };
+        worker.postMessage({
+            eval: "var C = NSObject.extend({}, { name: 'TNSWorkerNameClaim' }); " +
+                  "postMessage({ name: NSStringFromClass(C) });"
+        });
+    });
+
+    it("scopes worker-created TypeScript-extended class names to their isolate", function (done) {
+        var worker = new Worker("~/shared/Workers/EvalWorker.js");
+        worker.onmessage = function (msg) {
+            worker.terminate();
+            var workerName = msg.data.name;
+            expect(workerName).not.toBe("TNSWorkerTsNameClaim");
+            expect(workerName.indexOf("TNSWorkerTsNameClaim")).toBe(0);
+            done();
+        };
+        worker.postMessage({
+            eval: "function TNSWorkerTsNameClaim() {} " +
+                  "__extends(TNSWorkerTsNameClaim, NSObject); " +
+                  "postMessage({ name: NSStringFromClass(TNSWorkerTsNameClaim) });"
+        });
+    });
+});

TestRunner/app/tests/index.js

@@ -168,6 +168,9 @@ require("./InspectTests");
 // The ns:/node: builtin modules
 require("./NsUtilTests");
 
+// Worker-isolate scoping of extended objc class names
+require("./ExtendedClassNamingTests");
+
 // Tests common for all runtimes (git submodule of NativeScript/common-runtime-tests-app).
 require("../shared/index").runAllTests();