diff --git a/NativeScript/runtime/ClassBuilder.cpp b/NativeScript/runtime/ClassBuilder.cpp index 9195401e..4ca0fbf1 100644 --- a/NativeScript/runtime/ClassBuilder.cpp +++ b/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. diff --git a/NativeScript/runtime/ClassBuilder.h b/NativeScript/runtime/ClassBuilder.h index 0de3a3c9..8ab4a9f6 100644 --- a/NativeScript/runtime/ClassBuilder.h +++ b/NativeScript/runtime/ClassBuilder.h @@ -27,9 +27,9 @@ class ClassBuilder { public: static v8::Local 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 context); diff --git a/NativeScript/runtime/ClassBuilder.mm b/NativeScript/runtime/ClassBuilder.mm index 84ebe516..61a24d11 100644 --- a/NativeScript/runtime/ClassBuilder.mm +++ b/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 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)); diff --git a/TestRunner/app/tests/ExtendedClassNamingTests.js b/TestRunner/app/tests/ExtendedClassNamingTests.js new file mode 100644 index 00000000..79817f83 --- /dev/null +++ b/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) });" + }); + }); +}); diff --git a/TestRunner/app/tests/index.js b/TestRunner/app/tests/index.js index fe0e44ae..e1bb5f98 100644 --- a/TestRunner/app/tests/index.js +++ b/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();