diff --git a/include/v8.h b/include/v8.h index 0b5b4c6f79..c7d0204e73 100644 --- a/include/v8.h +++ b/include/v8.h @@ -6290,7 +6290,8 @@ class V8_EXPORT AccessorSignature : public Data { // --- Extensions --- - +V8_DEPRECATE_SOON("Implementation detail", + class ExternalOneByteStringResourceImpl); class V8_EXPORT ExternalOneByteStringResourceImpl : public String::ExternalOneByteStringResource { public: @@ -6317,7 +6318,7 @@ class V8_EXPORT Extension { // NOLINT int dep_count = 0, const char** deps = 0, int source_length = -1); - virtual ~Extension() { } + virtual ~Extension() { delete source_; } virtual Local GetNativeFunctionTemplate( Isolate* isolate, Local name) { return Local(); @@ -6326,7 +6327,8 @@ class V8_EXPORT Extension { // NOLINT const char* name() const { return name_; } size_t source_length() const { return source_length_; } const String::ExternalOneByteStringResource* source() const { - return &source_; } + return source_; + } int dependency_count() { return dep_count_; } const char** dependencies() { return deps_; } void set_auto_enable(bool value) { auto_enable_ = value; } @@ -6339,7 +6341,7 @@ class V8_EXPORT Extension { // NOLINT private: const char* name_; size_t source_length_; // expected to initialize before source_ - ExternalOneByteStringResourceImpl source_; + String::ExternalOneByteStringResource* source_; int dep_count_; const char** deps_; bool auto_enable_; diff --git a/src/api.cc b/src/api.cc index e336ad765f..800be40ce1 100644 --- a/src/api.cc +++ b/src/api.cc @@ -945,6 +945,21 @@ void RegisteredExtension::UnregisterAll() { first_extension_ = nullptr; } +namespace { +class ExtensionResource : public String::ExternalOneByteStringResource { + public: + ExtensionResource() : data_(0), length_(0) {} + ExtensionResource(const char* data, size_t length) + : data_(data), length_(length) {} + const char* data() const { return data_; } + size_t length() const { return length_; } + virtual void Dispose() {} + + private: + const char* data_; + size_t length_; +}; +} // anonymous namespace void RegisterExtension(Extension* that) { RegisteredExtension* extension = new RegisteredExtension(that); @@ -961,10 +976,10 @@ Extension::Extension(const char* name, source_length_(source_length >= 0 ? source_length : (source ? static_cast(strlen(source)) : 0)), - source_(source, source_length_), dep_count_(dep_count), deps_(deps), auto_enable_(false) { + source_ = new ExtensionResource(source, source_length_); CHECK(source != nullptr || source_length_ == 0); } @@ -6719,7 +6734,6 @@ MaybeLocal v8::String::NewExternalTwoByte( i::Handle string = i_isolate->factory() ->NewExternalStringFromTwoByte(resource) .ToHandleChecked(); - i_isolate->heap()->RegisterExternalString(*string); return Utils::ToLocal(string); } else { // The resource isn't going to be used, free it immediately. @@ -6743,7 +6757,6 @@ MaybeLocal v8::String::NewExternalOneByte( i::Handle string = i_isolate->factory() ->NewExternalStringFromOneByte(resource) .ToHandleChecked(); - i_isolate->heap()->RegisterExternalString(*string); return Utils::ToLocal(string); } else { // The resource isn't going to be used, free it immediately. @@ -6776,7 +6789,6 @@ bool v8::String::MakeExternal(v8::String::ExternalStringResource* resource) { DCHECK(!CanMakeExternal() || result); if (result) { DCHECK(obj->IsExternalString()); - isolate->heap()->RegisterExternalString(*obj); } return result; } @@ -6800,7 +6812,6 @@ bool v8::String::MakeExternal( DCHECK(!CanMakeExternal() || result); if (result) { DCHECK(obj->IsExternalString()); - isolate->heap()->RegisterExternalString(*obj); } return result; } diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc index 5ec3104ab4..226d290b35 100644 --- a/src/bootstrapper.cc +++ b/src/bootstrapper.cc @@ -81,7 +81,6 @@ Handle Bootstrapper::GetNativeSource(NativeType type, int index) { new NativesExternalStringResource(type, index); Handle source_code = isolate_->factory()->NewNativeSourceString(resource); - isolate_->heap()->RegisterExternalString(*source_code); DCHECK(source_code->is_short()); return source_code; } diff --git a/src/compiler-dispatcher/unoptimized-compile-job.cc b/src/compiler-dispatcher/unoptimized-compile-job.cc index 724c3f822f..8872e4817d 100644 --- a/src/compiler-dispatcher/unoptimized-compile-job.cc +++ b/src/compiler-dispatcher/unoptimized-compile-job.cc @@ -180,14 +180,12 @@ void UnoptimizedCompileJob::PrepareOnMainThread(Isolate* isolate) { if (source->IsOneByteRepresentation()) { ExternalOneByteString::Resource* resource = new OneByteWrapper(data, length); - source_wrapper_.reset(resource); wrapper = isolate->factory() ->NewExternalStringFromOneByte(resource) .ToHandleChecked(); } else { ExternalTwoByteString::Resource* resource = new TwoByteWrapper(data, length); - source_wrapper_.reset(resource); wrapper = isolate->factory() ->NewExternalStringFromTwoByte(resource) .ToHandleChecked(); diff --git a/src/compiler-dispatcher/unoptimized-compile-job.h b/src/compiler-dispatcher/unoptimized-compile-job.h index 8352e4e795..3e08388ca0 100644 --- a/src/compiler-dispatcher/unoptimized-compile-job.h +++ b/src/compiler-dispatcher/unoptimized-compile-job.h @@ -67,7 +67,6 @@ class V8_EXPORT_PRIVATE UnoptimizedCompileJob : public CompilerDispatcherJob { Handle shared_; // Global handle. Handle source_; // Global handle. Handle wrapper_; // Global handle. - std::unique_ptr source_wrapper_; size_t max_stack_size_; // Members required for parsing. diff --git a/src/extensions/externalize-string-extension.cc b/src/extensions/externalize-string-extension.cc index 54c6259b8b..363c0c593d 100644 --- a/src/extensions/externalize-string-extension.cc +++ b/src/extensions/externalize-string-extension.cc @@ -98,10 +98,6 @@ void ExternalizeStringExtension::Externalize( SimpleOneByteStringResource* resource = new SimpleOneByteStringResource( reinterpret_cast(data), string->length()); result = string->MakeExternal(resource); - if (result) { - i::Isolate* isolate = reinterpret_cast(args.GetIsolate()); - isolate->heap()->RegisterExternalString(*string); - } if (!result) delete resource; } else { uc16* data = new uc16[string->length()]; @@ -109,10 +105,6 @@ void ExternalizeStringExtension::Externalize( SimpleTwoByteStringResource* resource = new SimpleTwoByteStringResource( data, string->length()); result = string->MakeExternal(resource); - if (result) { - i::Isolate* isolate = reinterpret_cast(args.GetIsolate()); - isolate->heap()->RegisterExternalString(*string); - } if (!result) delete resource; } if (!result) { diff --git a/src/heap/factory.cc b/src/heap/factory.cc index 57e8bc1e60..b76234ebe9 100644 --- a/src/heap/factory.cc +++ b/src/heap/factory.cc @@ -1271,6 +1271,7 @@ MaybeHandle Factory::NewExternalStringFromOneByte( external_string->set_length(static_cast(length)); external_string->set_hash_field(String::kEmptyHashField); external_string->set_resource(resource); + isolate()->heap()->RegisterExternalString(*external_string); return external_string; } @@ -1303,6 +1304,7 @@ MaybeHandle Factory::NewExternalStringFromTwoByte( external_string->set_length(static_cast(length)); external_string->set_hash_field(String::kEmptyHashField); external_string->set_resource(resource); + isolate()->heap()->RegisterExternalString(*external_string); return external_string; } @@ -1318,6 +1320,7 @@ Handle Factory::NewNativeSourceString( external_string->set_length(static_cast(length)); external_string->set_hash_field(String::kEmptyHashField); external_string->set_resource(resource); + isolate()->heap()->RegisterExternalString(*external_string); return external_string; } diff --git a/src/objects.cc b/src/objects.cc index 07b13346fa..621bad39aa 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -2626,6 +2626,7 @@ bool String::MakeExternal(v8::String::ExternalStringResource* resource) { ExternalTwoByteString* self = ExternalTwoByteString::cast(this); self->set_resource(resource); + heap->RegisterExternalString(this); if (is_internalized) self->Hash(); // Force regeneration of the hash value. return true; } @@ -2696,6 +2697,7 @@ bool String::MakeExternal(v8::String::ExternalOneByteStringResource* resource) { ExternalOneByteString* self = ExternalOneByteString::cast(this); self->set_resource(resource); + heap->RegisterExternalString(this); if (is_internalized) self->Hash(); // Force regeneration of the hash value. return true; } diff --git a/test/cctest/heap/test-heap.cc b/test/cctest/heap/test-heap.cc index 3b37b5d666..1ecd64189e 100644 --- a/test/cctest/heap/test-heap.cc +++ b/test/cctest/heap/test-heap.cc @@ -5467,6 +5467,8 @@ TEST(Regress631969) { StaticOneByteResource external_string("12345678901234"); s3->MakeExternal(&external_string); CcTest::CollectGarbage(OLD_SPACE); + // This avoids the GC from trying to free stack allocated resources. + i::Handle::cast(s3)->set_resource(nullptr); } } diff --git a/test/cctest/parsing/test-scanner-streams.cc b/test/cctest/parsing/test-scanner-streams.cc index 1581d05a66..4657c12974 100644 --- a/test/cctest/parsing/test-scanner-streams.cc +++ b/test/cctest/parsing/test-scanner-streams.cc @@ -317,6 +317,11 @@ void TestCharacterStreams(const char* one_byte_source, unsigned length, std::unique_ptr uc16_stream( i::ScannerStream::For(uc16_string, start, end)); TestCharacterStream(one_byte_source, uc16_stream.get(), length, start, end); + + // This avoids the GC from trying to free a stack allocated resource. + if (uc16_string->IsExternalString()) + i::Handle::cast(uc16_string) + ->set_resource(nullptr); } // 1-byte external string @@ -333,6 +338,10 @@ void TestCharacterStreams(const char* one_byte_source, unsigned length, i::ScannerStream::For(ext_one_byte_string, start, end)); TestCharacterStream(one_byte_source, one_byte_stream.get(), length, start, end); + // This avoids the GC from trying to free a stack allocated resource. + if (ext_one_byte_string->IsExternalString()) + i::Handle::cast(ext_one_byte_string) + ->set_resource(nullptr); } // 1-byte generic i::String diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 834d6295ad..6abedf537e 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -15621,6 +15621,8 @@ class OneByteVectorResource : public v8::String::ExternalOneByteStringResource { virtual ~OneByteVectorResource() {} virtual size_t length() const { return data_.length(); } virtual const char* data() const { return data_.start(); } + virtual void Dispose() {} + private: i::Vector data_; }; @@ -15633,6 +15635,8 @@ class UC16VectorResource : public v8::String::ExternalStringResource { virtual ~UC16VectorResource() {} virtual size_t length() const { return data_.length(); } virtual const i::uc16* data() const { return data_.start(); } + virtual void Dispose() {} + private: i::Vector data_; }; @@ -15676,15 +15680,14 @@ THREADED_TEST(MorphCompositeStringTest) { OneByteVectorResource one_byte_resource( i::Vector(c_string, i::StrLength(c_string))); UC16VectorResource uc16_resource( - i::Vector(two_byte_string, - i::StrLength(c_string))); + i::Vector(two_byte_string, i::StrLength(c_string))); - Local lhs( - v8::Utils::ToLocal(factory->NewExternalStringFromOneByte( - &one_byte_resource).ToHandleChecked())); - Local rhs( - v8::Utils::ToLocal(factory->NewExternalStringFromOneByte( - &one_byte_resource).ToHandleChecked())); + Local lhs(v8::Utils::ToLocal( + factory->NewExternalStringFromOneByte(&one_byte_resource) + .ToHandleChecked())); + Local rhs(v8::Utils::ToLocal( + factory->NewExternalStringFromOneByte(&one_byte_resource) + .ToHandleChecked())); CHECK(env->Global()->Set(env.local(), v8_str("lhs"), lhs).FromJust()); CHECK(env->Global()->Set(env.local(), v8_str("rhs"), rhs).FromJust()); @@ -15697,10 +15700,10 @@ THREADED_TEST(MorphCompositeStringTest) { CHECK(lhs->IsOneByte()); CHECK(rhs->IsOneByte()); - MorphAString(*v8::Utils::OpenHandle(*lhs), &one_byte_resource, - &uc16_resource); - MorphAString(*v8::Utils::OpenHandle(*rhs), &one_byte_resource, - &uc16_resource); + i::String* ilhs = *v8::Utils::OpenHandle(*lhs); + i::String* irhs = *v8::Utils::OpenHandle(*rhs); + MorphAString(ilhs, &one_byte_resource, &uc16_resource); + MorphAString(irhs, &one_byte_resource, &uc16_resource); // This should UTF-8 without flattening, since everything is ASCII. Local cons = @@ -15743,6 +15746,16 @@ THREADED_TEST(MorphCompositeStringTest) { ->Get(env.local(), v8_str("slice_on_cons")) .ToLocalChecked()) .FromJust()); + + // This avoids the GC from trying to free a stack allocated resource. + if (ilhs->IsExternalOneByteString()) + i::ExternalOneByteString::cast(ilhs)->set_resource(nullptr); + else + i::ExternalTwoByteString::cast(ilhs)->set_resource(nullptr); + if (irhs->IsExternalOneByteString()) + i::ExternalOneByteString::cast(irhs)->set_resource(nullptr); + else + i::ExternalTwoByteString::cast(irhs)->set_resource(nullptr); } i::DeleteArray(two_byte_string); } @@ -20307,6 +20320,8 @@ THREADED_TEST(TwoByteStringInOneByteCons) { reresult = CompileRun("str2.charCodeAt(2);"); CHECK_EQ(static_cast('e'), reresult->Int32Value(context.local()).FromJust()); + // This avoids the GC from trying to free stack allocated resources. + i::Handle::cast(flat_string)->set_resource(nullptr); } diff --git a/test/cctest/test-serialize.cc b/test/cctest/test-serialize.cc index 56bd87a495..1f3f39c7d8 100644 --- a/test/cctest/test-serialize.cc +++ b/test/cctest/test-serialize.cc @@ -1877,6 +1877,11 @@ TEST(CodeSerializerExternalString) { CHECK_EQ(15.0, copy_result->Number()); + // This avoids the GC from trying to free stack allocated resources. + i::Handle::cast(one_byte_string) + ->set_resource(nullptr); + i::Handle::cast(two_byte_string) + ->set_resource(nullptr); delete cache; } @@ -1933,6 +1938,8 @@ TEST(CodeSerializerLargeExternalString) { CHECK_EQ(42.0, copy_result->Number()); + // This avoids the GC from trying to free stack allocated resources. + i::Handle::cast(name)->set_resource(nullptr); delete cache; string.Dispose(); } @@ -1982,6 +1989,8 @@ TEST(CodeSerializerExternalScriptName) { CHECK_EQ(10.0, copy_result->Number()); + // This avoids the GC from trying to free stack allocated resources. + i::Handle::cast(name)->set_resource(nullptr); delete cache; } diff --git a/test/cctest/test-strings.cc b/test/cctest/test-strings.cc index 589c3bc481..8657c481d3 100644 --- a/test/cctest/test-strings.cc +++ b/test/cctest/test-strings.cc @@ -1244,6 +1244,8 @@ TEST(SliceFromExternal) { CHECK_EQ(SlicedString::cast(*slice)->parent(), *string); CHECK(SlicedString::cast(*slice)->parent()->IsExternalString()); CHECK(slice->IsFlat()); + // This avoids the GC from trying to free stack allocated resources. + i::Handle::cast(string)->set_resource(nullptr); } diff --git a/test/unittests/compiler-dispatcher/unoptimized-compile-job-unittest.cc b/test/unittests/compiler-dispatcher/unoptimized-compile-job-unittest.cc index 4f431cd09d..f7fb335ac6 100644 --- a/test/unittests/compiler-dispatcher/unoptimized-compile-job-unittest.cc +++ b/test/unittests/compiler-dispatcher/unoptimized-compile-job-unittest.cc @@ -88,9 +88,9 @@ TEST_F(UnoptimizedCompileJobTest, StateTransitions) { } TEST_F(UnoptimizedCompileJobTest, SyntaxError) { - test::ScriptResource script("^^^", strlen("^^^")); + test::ScriptResource* script = new test::ScriptResource("^^^", strlen("^^^")); std::unique_ptr job(new UnoptimizedCompileJob( - isolate(), tracer(), test::CreateSharedFunctionInfo(isolate(), &script), + isolate(), tracer(), test::CreateSharedFunctionInfo(isolate(), script), FLAG_stack_size)); job->PrepareOnMainThread(isolate()); @@ -148,9 +148,10 @@ TEST_F(UnoptimizedCompileJobTest, CompileFailureToAnalyse) { raw_script += "'x' + 'x' - "; } raw_script += " 'x'; }"; - test::ScriptResource script(raw_script.c_str(), strlen(raw_script.c_str())); + test::ScriptResource* script = + new test::ScriptResource(raw_script.c_str(), strlen(raw_script.c_str())); std::unique_ptr job(new UnoptimizedCompileJob( - isolate(), tracer(), test::CreateSharedFunctionInfo(isolate(), &script), + isolate(), tracer(), test::CreateSharedFunctionInfo(isolate(), script), 100)); job->PrepareOnMainThread(isolate()); @@ -174,9 +175,10 @@ TEST_F(UnoptimizedCompileJobTest, CompileFailureToFinalize) { raw_script += "'x' + 'x' - "; } raw_script += " 'x'; }"; - test::ScriptResource script(raw_script.c_str(), strlen(raw_script.c_str())); + test::ScriptResource* script = + new test::ScriptResource(raw_script.c_str(), strlen(raw_script.c_str())); std::unique_ptr job(new UnoptimizedCompileJob( - isolate(), tracer(), test::CreateSharedFunctionInfo(isolate(), &script), + isolate(), tracer(), test::CreateSharedFunctionInfo(isolate(), script), 50)); job->PrepareOnMainThread(isolate()); @@ -219,9 +221,10 @@ TEST_F(UnoptimizedCompileJobTest, CompileOnBackgroundThread) { " var d = { foo: 100, bar : bar() }\n" " return bar;" "}"; - test::ScriptResource script(raw_script, strlen(raw_script)); + test::ScriptResource* script = + new test::ScriptResource(raw_script, strlen(raw_script)); std::unique_ptr job(new UnoptimizedCompileJob( - isolate(), tracer(), test::CreateSharedFunctionInfo(isolate(), &script), + isolate(), tracer(), test::CreateSharedFunctionInfo(isolate(), script), 100)); job->PrepareOnMainThread(isolate());