use null prototype for opaque wrappers

[anonrig]

This is important for passing several web-platform tests related to streams and fetch.

We use null prototype to prevent Object.prototype.then patches from intercepting internal promise operations (required for WPT compliance).

[jasnell]

An alternative way of doing this that might be better, is to add the following when the opaqueTemplate is created:

auto protoTemplate = v8::FunctionTemplate::New(ptr, &throwIllegalConstructor);
protoTemplate->RemovePrototype();
opaqueTemplate->SetPrototypeProviderTemplate(protoTemplate);

This would avoid the added branch and after-creation prototype mutation in the hotpath and allows the wpt's to pass. I haven't benchmarked to see if it actually is faster tho but it's worth an experiment.

[anonrig]

@jasnell I believe you're right and I think it is actually faster than the current state.

[jasnell]

LGTM. I do think it might be beneficial for either the PR or commit message to explain a bit about WHY this fixes the tests as it's non-obvious, but that's not necessarily blocking.

[anonrig]

@jasnell SetPrototypeProviderTemplate with RemovePrototype() is failing with a weird v8 error on debug mode. I'm investigating to see if there is a better solution here.

[anonrig]

For reference here's the error occuring only on debug build:

workerd/jsg/setup.c++:38: fatal: V8 fatal error; location = external/+_repo_rules2+v8/src/api/api-natives.cc:689; message = Holder<To> v8::internal::TrustedCast(Holder<From>, SourceLocation) [To = v8::internal::JSObject, From = v8::internal::Object, Holder = v8::internal::DirectHandle]
*** Received signal #6: Aborted
stack: /lib/x86_64-linux-gnu/libc.so.6@9eb2b /lib/x86_64-linux-gnu/libc.so.6@4527d /lib/x86_64-linux-gnu/libc.so.6@288fe src/workerd/server/workerd@65c8781 src/workerd/server/workerd@65c5592 src/workerd/server/workerd@a94217c src/workerd/server/workerd@6b5faa4 src/workerd/server/workerd@6acf9cc src/workerd/server/workerd@6e71c76 src/workerd/server/workerd@6e6eee5 src/workerd/server/workerd@6e6f22a src/workerd/server/workerd@6e6f68d src/workerd/server/workerd@6e6f283 src/workerd/server/workerd@6e0440f src/workerd/server/workerd@65e4f06 src/workerd/server/workerd@492aaac src/workerd/server/workerd@492a88a src/workerd/server/workerd@492a808 src/workerd/server/workerd@4929881 src/workerd/server/workerd@58342b1 src/workerd/server/workerd@5834081 src/workerd/server/workerd@5859cc3 src/workerd/server/workerd@5859896 src/workerd/server/workerd@5856e29 src/workerd/server/workerd@5856708 src/workerd/server/workerd@585653b src/workerd/server/workerd@5832646 src/workerd/server/workerd@58322da src/workerd/server/workerd@584a21b
__pthread_kill_implementation
pthread_kill.c:43:17
__pthread_kill_internal
pthread_kill.c:78:10
pthread_kill
pthread_kill.c:89:10

raise
../sysdeps/posix/raise.c:26:13

abort
abort.c:79:7

workerd::jsg::reportV8FatalError(kj::StringPtr, kj::StringPtr)
src/workerd/jsg/setup.c++:39:5

workerd::jsg::v8DcheckError(char const*, int, char const*)
src/workerd/jsg/setup.c++:45:3

V8_Dcheck(char const*, int, char const*)
external/+_repo_rules2+v8/src/base/logging.cc:244:3

T1<T> v8::internal::TrustedCast<v8::internal::JSObject, v8::internal::Object, v8::internal::DirectHandle>(T1<T0>, v8::SourceLocation)
external/+_repo_rules2+v8/src/objects/casting.h:156:3

T1<T> v8::internal::Cast<v8::internal::JSObject, v8::internal::Object, v8::internal::DirectHandle>(T1<T0>, v8::SourceLocation)
external/+_repo_rules2+v8/src/objects/casting.h:209:10

v8::internal::ApiNatives::CreateApiFunction(v8::internal::Isolate*, v8::internal::DirectHandle<v8::internal::NativeContext>, v8::internal::DirectHandle<v8::internal::FunctionTemplateInfo>, v8::internal::DirectHandle<v8::internal::Object>, v8::internal::InstanceType, v8::internal::MaybeDirectHandle<v8::internal::Name>)
external/+_repo_rules2+v8/src/api/api-natives.cc:689:51

v8::internal::(anonymous namespace)::InstantiateFunction(v8::internal::Isolate*, v8::internal::DirectHandle<v8::internal::NativeContext>, v8::internal::DirectHandle<v8::internal::FunctionTemplateInfo>, v8::internal::MaybeDirectHandle<v8::internal::Name>)
external/+_repo_rules2+v8/src/api/api-natives.cc:440:33

v8::internal::(anonymous namespace)::InstantiateFunction(v8::internal::Isolate*, v8::internal::DirectHandle<v8::internal::FunctionTemplateInfo>, v8::internal::MaybeDirectHandle<v8::internal::Name>)
external/+_repo_rules2+v8/src/api/api-natives.cc:54:10

v8::internal::(anonymous namespace)::InstantiateObject(v8::internal::Isolate*, v8::internal::DirectHandle<v8::internal::ObjectTemplateInfo>, v8::internal::DirectHandle<v8::internal::JSReceiver>, bool)
external/+_repo_rules2+v8/src/api/api-natives.cc:323:7

v8::internal::ApiNatives::InstantiateObject(v8::internal::Isolate*, v8::internal::DirectHandle<v8::internal::ObjectTemplateInfo>, v8::internal::DirectHandle<v8::internal::JSReceiver>)
external/+_repo_rules2+v8/src/api/api-natives.cc:514:10

v8::ObjectTemplate::NewInstance(v8::Local<v8::Context>)
external/+_repo_rules2+v8/src/api/api.cc:7217:7

workerd::jsg::Wrappable::attachOpaqueWrapper(v8::Local<v8::Context>, bool)
src/workerd/jsg/wrappable.c++:394:79

v8::Local<v8::Function> workerd::jsg::FunctionWrapper<workerd::server::(anonymous namespace)::JsgWorkerdIsolate_TypeWrapper>::wrap<void (v8::FunctionCallbackInfo<v8::Value> const&)>(workerd::jsg::Lock&, v8::Local<v8::Context>, kj::Maybe<v8::Local<v8::Object>>, workerd::jsg::Function<void (v8::FunctionCallbackInfo<v8::Value> const&)>&&)::'lambda'(workerd::jsg::Ref<workerd::jsg::WrappableFunction<void (v8::FunctionCallbackInfo<v8::Value> const&)>>&)::operator()(workerd::jsg::Ref<workerd::jsg::WrappableFunction<void (v8::FunctionCallbackInfo<v8::Value> const&)>>&) const
bazel-out/k8-dbg/bin/src/workerd/jsg/_virtual_includes/jsg-core/workerd/jsg/function.h:359:21

v8::Local<v8::Function> workerd::jsg::Function<void (v8::FunctionCallbackInfo<v8::Value> const&)>::getOrCreateHandle<v8::Local<v8::Function> workerd::jsg::FunctionWrapper<workerd::server::(anonymous namespace)::JsgWorkerdIsolate_TypeWrapper>::wrap<void (v8::FunctionCallbackInfo<v8::Value> const&)>(workerd::jsg::Lock&, v8::Local<v8::Context>, kj::Maybe<v8::Local<v8::Object>>, workerd::jsg::Function<void (v8::FunctionCallbackInfo<v8::Value> const&)>&&)::'lambda'(workerd::jsg::Ref<workerd::jsg::WrappableFunction<void (v8::FunctionCallbackInfo<v8::Value> const&)>>&)>(v8::Isolate*, void (&&)(v8::FunctionCallbackInfo<v8::Value> const&))
bazel-out/k8-dbg/bin/src/workerd/jsg/_virtual_includes/jsg-core/workerd/jsg/function.h:192:16

v8::Local<v8::Function> workerd::jsg::FunctionWrapper<workerd::server::(anonymous namespace)::JsgWorkerdIsolate_TypeWrapper>::wrap<void (v8::FunctionCallbackInfo<v8::Value> const&)>(workerd::jsg::Lock&, v8::Local<v8::Context>, kj::Maybe<v8::Local<v8::Object>>, workerd::jsg::Function<void (v8::FunctionCallbackInfo<v8::Value> const&)>&&)
bazel-out/k8-dbg/bin/src/workerd/jsg/_virtual_includes/jsg-core/workerd/jsg/function.h:331:17

workerd::jsg::Isolate<workerd::server::(anonymous namespace)::JsgWorkerdIsolate_TypeWrapper>::Lock::wrapSimpleFunction(v8::Local<v8::Context>, workerd::jsg::Function<void (v8::FunctionCallbackInfo<v8::Value> const&)>)
bazel-out/k8-dbg/bin/src/workerd/jsg/_virtual_includes/jsg-core/workerd/jsg/setup.h:724:55

workerd::Worker::setupContext(workerd::jsg::Lock&, v8::Local<v8::Context>, workerd::Worker::LoggingOptions const&)::$_0::operator()(char const*, capnp::schemas::Level_a8b8c4ab94a9dea7) const
src/workerd/io/worker.c++:1640:19

workerd::Worker::setupContext(workerd::jsg::Lock&, v8::Local<v8::Context>, workerd::Worker::LoggingOptions const&)
src/workerd/io/worker.c++:1648:3

workerd::Worker::Isolate::Impl::Lock::setupContext(v8::Local<v8::Context>)
src/workerd/io/worker.c++:643:7

workerd::Worker::Script::Script(kj::Own<workerd::Worker::Isolate const, std::nullptr_t>, kj::StringPtr, workerd::WorkerSource const&, workerd::IsolateObserver::StartType, bool, kj::Maybe<workerd::Worker::ValidationErrorReporter&>, kj::Maybe<kj::Own<workerd::api::pyodide::ArtifactBundler_State, std::nullptr_t>>, workerd::SpanParent, kj::Own<workerd::VirtualFileSystem, std::nullptr_t>, kj::Maybe<kj::Arc<workerd::jsg::modules::ModuleRegistry>>)::$_0::operator()(workerd::jsg::V8StackScope&) const::'lambda0'()::operator()() const
src/workerd/io/worker.c++:1379:22

auto workerd::jsg::Lock::withinHandleScope<workerd::Worker::Script::Script(kj::Own<workerd::Worker::Isolate const, std::nullptr_t>, kj::StringPtr, workerd::WorkerSource const&, workerd::IsolateObserver::StartType, bool, kj::Maybe<workerd::Worker::ValidationErrorReporter&>, kj::Maybe<kj::Own<workerd::api::pyodide::ArtifactBundler_State, std::nullptr_t>>, workerd::SpanParent, kj::Own<workerd::VirtualFileSystem, std::nullptr_t>, kj::Maybe<kj::Arc<workerd::jsg::modules::ModuleRegistry>>)::$_0::operator()(workerd::jsg::V8StackScope&) const::'lambda0'()>(workerd::Worker::Script::Script(kj::Own<workerd::Worker::Isolate const, std::nullptr_t>, kj::StringPtr, workerd::WorkerSource const&, workerd::IsolateObserver::StartType, bool, kj::Maybe<workerd::Worker::ValidationErrorReporter&>, kj::Maybe<kj::Own<workerd::api::pyodide::ArtifactBundler_State, std::nullptr_t>>, workerd::SpanParent, kj::Own<workerd::VirtualFileSystem, std::nullptr_t>, kj::Maybe<kj::Arc<workerd::jsg::modules::ModuleRegistry>>)::$_0::operator()(workerd::jsg::V8StackScope&) const::'lambda0'()&&)
bazel-out/k8-dbg/bin/src/workerd/jsg/_virtual_includes/jsg-core/workerd/jsg/jsg.h:2689:14

workerd::Worker::Script::Script(kj::Own<workerd::Worker::Isolate const, std::nullptr_t>, kj::StringPtr, workerd::WorkerSource const&, workerd::IsolateObserver::StartType, bool, kj::Maybe<workerd::Worker::ValidationErrorReporter&>, kj::Maybe<kj::Own<workerd::api::pyodide::ArtifactBundler_State, std::nullptr_t>>, workerd::SpanParent, kj::Own<workerd::VirtualFileSystem, std::nullptr_t>, kj::Maybe<kj::Arc<workerd::jsg::modules::ModuleRegistry>>)::$_0::operator()(workerd::jsg::V8StackScope&) const
src/workerd/io/worker.c++:1364:10

auto workerd::jsg::V8StackScope::runInV8StackImpl<workerd::Worker::Script::Script(kj::Own<workerd::Worker::Isolate const, std::nullptr_t>, kj::StringPtr, workerd::WorkerSource const&, workerd::IsolateObserver::StartType, bool, kj::Maybe<workerd::Worker::ValidationErrorReporter&>, kj::Maybe<kj::Own<workerd::api::pyodide::ArtifactBundler_State, std::nullptr_t>>, workerd::SpanParent, kj::Own<workerd::VirtualFileSystem, std::nullptr_t>, kj::Maybe<kj::Arc<workerd::jsg::modules::ModuleRegistry>>)::$_0>(void*, workerd::Worker::Script::Script(kj::Own<workerd::Worker::Isolate const, std::nullptr_t>, kj::StringPtr, workerd::WorkerSource const&, workerd::IsolateObserver::StartType, bool, kj::Maybe<workerd::Worker::ValidationErrorReporter&>, kj::Maybe<kj::Own<workerd::api::pyodide::ArtifactBundler_State, std::nullptr_t>>, workerd::SpanParent, kj::Own<workerd::VirtualFileSystem, std::nullptr_t>, kj::Maybe<kj::Arc<workerd::jsg::modules::ModuleRegistry>>)::$_0)
bazel-out/k8-dbg/bin/src/workerd/jsg/_virtual_includes/jsg-core/workerd/jsg/jsg.h:2907:12

auto workerd::jsg::runInV8Stack<workerd::Worker::Script::Script(kj::Own<workerd::Worker::Isolate const, std::nullptr_t>, kj::StringPtr, workerd::WorkerSource const&, workerd::IsolateObserver::StartType, bool, kj::Maybe<workerd::Worker::ValidationErrorReporter&>, kj::Maybe<kj::Own<workerd::api::pyodide::ArtifactBundler_State, std::nullptr_t>>, workerd::SpanParent, kj::Own<workerd::VirtualFileSystem, std::nullptr_t>, kj::Maybe<kj::Arc<workerd::jsg::modules::ModuleRegistry>>)::$_0>(workerd::Worker::Script::Script(kj::Own<workerd::Worker::Isolate const, std::nullptr_t>, kj::StringPtr, workerd::WorkerSource const&, workerd::IsolateObserver::StartType, bool, kj::Maybe<workerd::Worker::ValidationErrorReporter&>, kj::Maybe<kj::Own<workerd::api::pyodide::ArtifactBundler_State, std::nullptr_t>>, workerd::SpanParent, kj::Own<workerd::VirtualFileSystem, std::nullptr_t>, kj::Maybe<kj::Arc<workerd::jsg::modules::ModuleRegistry>>)::$_0)
bazel-out/k8-dbg/bin/src/workerd/jsg/_virtual_includes/jsg-core/workerd/jsg/jsg.h:2919:10

workerd::Worker::Script::Script(kj::Own<workerd::Worker::Isolate const, std::nullptr_t>, kj::StringPtr, workerd::WorkerSource const&, workerd::IsolateObserver::StartType, bool, kj::Maybe<workerd::Worker::ValidationErrorReporter&>, kj::Maybe<kj::Own<workerd::api::pyodide::ArtifactBundler_State, std::nullptr_t>>, workerd::SpanParent, kj::Own<workerd::VirtualFileSystem, std::nullptr_t>, kj::Maybe<kj::Arc<workerd::jsg::modules::ModuleRegistry>>)
src/workerd/io/worker.c++:1349:3

kj::Own<workerd::Worker::Script, std::nullptr_t> kj::atomicRefcounted<workerd::Worker::Script, kj::Own<workerd::Worker::Isolate const, std::nullptr_t>, kj::StringPtr&, workerd::WorkerSource const&, workerd::IsolateObserver::StartType&, bool&, kj::Maybe<workerd::Worker::ValidationErrorReporter&>&, kj::Maybe<kj::Own<workerd::api::pyodide::ArtifactBundler_State, std::nullptr_t>>, workerd::SpanParent, kj::Own<workerd::VirtualFileSystem, std::nullptr_t>, kj::Maybe<kj::Arc<workerd::jsg::modules::ModuleRegistry>>>(kj::Own<workerd::Worker::Isolate const, std::nullptr_t>&&, kj::StringPtr&, workerd::WorkerSource const&, workerd::IsolateObserver::StartType&, bool&, kj::Maybe<workerd::Worker::ValidationErrorReporter&>&, kj::Maybe<kj::Own<workerd::api::pyodide::ArtifactBundler_State, std::nullptr_t>>&&, workerd::SpanParent&&, kj::Own<workerd::VirtualFileSystem, std::nullptr_t>&&, kj::Maybe<kj::Arc<workerd::jsg::modules::ModuleRegistry>>&&)
bazel-out/k8-dbg/bin/external/+http+capnp-cpp/src/kj/_virtual_includes/kj/kj/refcount.h:348:47

[jasnell]

@erikcorry @dcarney-cf

[anonrig]

Ok this a valid v8 bug. I'll open an upstream fix.

[anonrig]

Here's the fix: https://chromium-review.googlesource.com/c/v8/v8/+/7323287