diff --git a/.github/workflows/major-release.yml b/.github/workflows/major-release.yml index b65917f89e74..1b15a00c8df1 100644 --- a/.github/workflows/major-release.yml +++ b/.github/workflows/major-release.yml @@ -2,7 +2,7 @@ name: Major Release on: schedule: - - cron: 0 0 15 2,8 * # runs at midnight UTC every 15 February and 15 August + - cron: 0 0 15 2 * # runs at midnight UTC every 15 February permissions: contents: read diff --git a/doc/contributing/releases.md b/doc/contributing/releases.md index c6b51bc2acff..da61b360ac9f 100644 --- a/doc/contributing/releases.md +++ b/doc/contributing/releases.md @@ -422,6 +422,11 @@ already defined in `src/node_version.h`: #define NODE_MAJOR_VERSION x #define NODE_MINOR_VERSION y #define NODE_PATCH_VERSION z + +// And for alpha releases: +#define NODE_ALPHA_MAJOR_VERSION a +#define NODE_ALPHA_MINOR_VERSION b +#define NODE_ALPHA_PATCH_VERSION c ``` Set the `NODE_VERSION_IS_RELEASE` macro value to `1`. This causes the build to @@ -431,6 +436,14 @@ be produced with a version string that does not have a trailing pre-release tag: #define NODE_VERSION_IS_RELEASE 1 ``` +
+Major version release + +Remove the `NODE_ALPHA_MAJOR_VERSION`, `NODE_ALPHA_MINOR_VERSION`, and +`NODE_ALPHA_PATCH_VERSION` macros. + +
+ ### 4. Update the changelog _(This step will be done automatically if you are using `create-release-proposal` or `git node release --prepare`)_ @@ -900,8 +913,8 @@ project README. On release proposal branch, edit `src/node_version.h` again and: -* Increment `NODE_PATCH_VERSION` by one -* Change `NODE_VERSION_IS_RELEASE` back to `0` +* Increment `NODE_PATCH_VERSION` (or `NODE_ALPHA_PATCH_VERSION` for alpha releases) by one. +* Change `NODE_VERSION_IS_RELEASE` back to `0`. Commit this change with the following commit message format: @@ -968,9 +981,12 @@ git restore --source=upstream/main src/node_version.h On the main branch, instead of reverting changes made to `src/node_version.h` edit it instead and: -* Increment `NODE_MAJOR_VERSION` by one -* Reset `NODE_PATCH_VERSION` to `0` -* Change `NODE_VERSION_IS_RELEASE` back to `0` +* Increment `NODE_MAJOR_VERSION` by one. +* Reset `NODE_PATCH_VERSION` and `NODE_MINOR_VERSION` to `0`. +* Set `NODE_ALPHA_MAJOR_VERSION`, `NODE_ALPHA_MINOR_VERSION`, and + `NODE_ALPHA_PATCH_VERSION` back to `0` (`main` should already have this, the + release commit will have them removed). +* Change `NODE_VERSION_IS_RELEASE` back to `0`. Amend the current commit to apply the changes: @@ -1281,9 +1297,9 @@ git node release --prepare --startLTS To mark a release line as LTS, the following changes must be made to `src/node_version.h`: -* The `NODE_MINOR_VERSION` macro must be incremented by one -* The `NODE_PATCH_VERSION` macro must be set to `0` -* The `NODE_VERSION_IS_LTS` macro must be set to `1` +* The `NODE_MINOR_VERSION` macro must be incremented by one. +* The `NODE_PATCH_VERSION` macro must be set to `0`. +* The `NODE_VERSION_IS_LTS` macro must be set to `1`. * The `NODE_VERSION_LTS_CODENAME` macro must be set to the code name selected for the LTS release. @@ -1352,15 +1368,15 @@ from cutting a minor or patch release. ### Schedule -New Node.js Major releases happen twice per year: +New Node.js Major releases happen once per year: -* Even-numbered releases are cut in April. -* Odd-numbered releases are cut in October. +* Branch-off is in October. +* Semver-major release is in April. Major releases should be targeted for the third Tuesday of the release month. A major release must not slip beyond the release month. In other words, major -releases must not slip into May or November. +releases must not slip into May. The @nodejs/releasers make a call for releasers 3 months in advance. Currently, this call is automated in the `#nodejs-release-private` @@ -1370,15 +1386,15 @@ The release date for the next major release should be announced immediately following the current release (e.g. the release date for 13.0.0 should be announced immediately following the release of 12.0.0). -### Release branch +### Branch-off (October) -Approximately two months before a major release, new `vN.x` and -`vN.x-staging` branches (where `N` indicates the major release) should be -created as forks of the `main` branch. Up until the cut-off date announced by -the releaser, these must be kept in sync with `main`. +#### Release branch -The `vN.x` and `vN.x-staging` branches must be kept in sync with one another -up until the date of the release. +Approximately six months before a major release, new `vN.x` and +`vN.x-staging` branches (where `N` indicates the major release) should be +created as forks of the `main` branch. Alpha releases should be released picking +up commits from `main`. Target the first alpha release to be released the same +day as the previous release line is graduated to LTS status. If a `SEMVER-MAJOR` pull request lands on the default branch within one month prior to the major release date, it must not be included on the new major @@ -1386,10 +1402,9 @@ staging branch, unless there is consensus from the Node.js releasers team to do so. This measure aims to ensure better stability for the release candidate (RC) phase, which begins approximately two weeks prior to the official release. By restricting `SEMVER-MAJOR` commits in this period, we provide more time for -thorough testing and reduce the potential for major breakages, especially in -LTS lines. +thorough testing and reduce the potential for major breakages. -### Create release labels +#### Create release labels The following issue labels must be created: @@ -1404,9 +1419,9 @@ The label description can be copied from existing labels of previous releases. The label color must be the same for all new labels, but different from the labels of previous releases. -### Release proposal +#### Initial Alpha release proposal -A draft release proposal should be created 6 weeks before the release. A +A draft release proposal should be created before the release. A separate `vN.x-proposal` branch should be created that tracks the `vN.x` branch. This branch will contain the draft release commit (with the draft changelog). @@ -1414,21 +1429,7 @@ changelog). Notify the `@nodejs/npm` team in the release proposal PR to inform them of the upcoming release. -To keep the branch in sync until the release date, it can be as simple as -doing the following: - -> Make sure to check that there are no PRs with the label `dont-land-on-vX.x`. - -```bash -git checkout vN.x -git reset --hard upstream/main -git checkout vN.x-staging -git reset --hard upstream/main -git push upstream vN.x -git push upstream vN.x-staging -``` - -### Update `NODE_MODULE_VERSION` +##### Update `NODE_MODULE_VERSION` This macro in `src/node_version.h` is used to signal an ABI version for native addons. It currently has two common uses in the community: @@ -1458,24 +1459,12 @@ see a need to bump `NODE_MODULE_VERSION` outside of a major release then you should consult the TSC. Commits may need to be reverted or a major version bump may need to happen. -### Test releases and release candidates - -Test builds should be generated from the `vN.x-proposal` branch starting at -about 6 weeks before the release. - -Release Candidates should be generated from the `vN.x-proposal` branch starting -at about 4 weeks before the release, with a target of one release candidate -per week. - -Always run test releases and release candidates through the Canary in the -Goldmine tool for additional testing. - -### Changelogs +##### Changelogs Generating major release changelogs is a bit more involved than minor and patch changelogs. -#### Create the changelog file +###### Create the changelog file In the `doc/changelogs` directory, create a new `CHANGELOG_V{N}.md` file where `{N}` is the major version of the release. Follow the structure of the existing @@ -1487,7 +1476,7 @@ updated to account for the new `CHANGELOG_V{N}.md` file. Once the file is created, the root `CHANGELOG.md` file must be updated to reference the newly-created major release `CHANGELOG_V{N}.md`. -#### Generate the changelog +###### Generate the changelog To generate a proper major release changelog, use the `branch-diff` tool to compare the `vN.x` branch against the `vN-1.x` branch (e.g. for Node.js 12.0, @@ -1506,14 +1495,7 @@ $ branch-diff upstream/vN-1.x upstream/vN.x --require-label=semver-minor --group $ branch-diff upstream/vN-1.x upstream/vN.x --exclude-label=semver-major,semver-minor --group --filter-release --markdown # get all patches ``` -#### Generate the notable changes - -For a major release, all SEMVER-MAJOR commits that are not strictly internal, -test, or doc-related are to be listed as notable changes. Some SEMVER-MINOR -commits may be listed as notable changes on a case-by-case basis. Use your -judgment there. - -### Update the expected assets +##### Update the expected assets The promotion script does a basic check that the expected files are present. Open a pull request in the Build repository to add the list of expected files @@ -1522,6 +1504,39 @@ version of the release), in the [expected assets][] folder. The change will need to be deployed onto the web server by a member of the [build-infra team][] before the release is promoted. +### Semver-major release (April) + +#### Release proposal + +A draft release proposal should be created 6 weeks before the release. A +separate `vN.x-proposal` branch should be created that tracks the `vN.x` +branch. This branch will contain the draft release commit (with the draft +changelog). + +Notify the `@nodejs/npm` team in the release proposal PR to inform them of the +upcoming release. + +Major release proposal should contain a single commit, the release one. All +semver-major changes must have landed in a alpha version before the major is +released. Semver-major changes that have missed the alpha period will be included +in the next major release line. + +##### Marking a release line as "out of Alpha" + +To mark a release line as stable, the following changes must be made to +`src/node_version.h`: + +* Remove `NODE_ALPHA_MAJOR_VERSION`, `NODE_ALPHA_MINOR_VERSION`, and + `NODE_ALPHA_PATCH_VERSION`. + +#### Generate the notable changes + +For a major release, all SEMVER-MAJOR commits that are not strictly internal, +test, or doc-related are to be listed as notable changes. Some SEMVER-MINOR +commits may be listed as notable changes on a case-by-case basis. Use your +judgment there. +Include the notable changes from the Alpha versions where it applies. + ### Snap The Node.js [Snap][] package has a "default" for installs where the user hasn't diff --git a/lib/internal/ffi/fast-api.js b/lib/internal/ffi/fast-api.js index 486a119a2e07..a232caa1af24 100644 --- a/lib/internal/ffi/fast-api.js +++ b/lib/internal/ffi/fast-api.js @@ -24,6 +24,7 @@ const { getRawPointer, kFastArguments, kFastBufferInvoke, + uintptrMax, } = internalBinding('ffi'); const { @@ -110,6 +111,14 @@ function needsPointerConversion(type) { needsNullPointerConversion(type) || needsStringPointerConversion(type); } +function validateFastPointerArg(type, value, index) { + if (needsPointerConversion(type) && typeof value === 'bigint' && + (value < 0n || value > uintptrMax)) { + throwFFIArgError( + `Argument ${index} must be a non-negative pointer bigint`); + } +} + function hasStringPointerArg(type, value) { return typeof value === 'string' && needsStringPointerConversion(type); } @@ -159,6 +168,7 @@ function getStringConversionPointer(state, value, index) { } function convertPointerArg(type, value, stringState, index) { + validateFastPointerArg(type, value, index); if (needsNullPointerConversion(type) && (value === null || value === undefined)) { return 0n; @@ -261,6 +271,7 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) { throwFFIArgCountError(1, arguments.length); } validateFastIntegerArg(t0, a0, 0); + validateFastPointerArg(t0, a0, 0); let arg = a0; if (needsNullPointerConversion(t0) && (arg === null || arg === undefined)) { diff --git a/src/node.h b/src/node.h index 32a9806ae848..bf6537e3bfe6 100644 --- a/src/node.h +++ b/src/node.h @@ -844,6 +844,16 @@ NODE_EXTERN void SetProcessExitHandler( std::function&& handler); NODE_EXTERN void DefaultProcessExitHandler(Environment* env, int exit_code); +// Sets a process-global handler invoked when Node.js programmatically aborts. +// Nullable strings representing the location and reason for the abort may or +// may not be passed as a parameter to the handler. The handler should not +// return, but node will ensure that the process exits after the handler is +// called regardless of whether or not it returns. Passing nullptr restores the +// default handler. This is process-global and may be invoked before any Isolate +// or Environment exists. +using AbortHandler = void (*)(const char* location, const char* message); +NODE_EXTERN void SetAbortHandler(AbortHandler handler); + // This may return nullptr if context is not associated with a Node instance. NODE_EXTERN Environment* GetCurrentEnvironment(v8::Local context); NODE_EXTERN IsolateData* GetEnvironmentIsolateData(Environment* env); diff --git a/src/node_errors.cc b/src/node_errors.cc index 63db97f6a56d..2c464bb895e5 100644 --- a/src/node_errors.cc +++ b/src/node_errors.cc @@ -393,6 +393,30 @@ void AppendExceptionLine(Environment* env, .FromMaybe(false)); } +namespace { +// Default handler: Dumps native + JS backtraces to stderr and exits. This +// indirectly calls backtrace so it can not be marked as [[noreturn]] (see the +// comment on node::Assert() below). `message` and `location` are ignored +// because the assertion/fatal-error message, if any, is already printed to +// stderr by the caller (Assert()/OnFatalError()) before this handler runs. +void DefaultAbortHandler(const char* /*location*/, const char* /*message*/) { + DumpNativeBacktrace(stderr); + DumpJavaScriptBacktrace(stderr); + fflush(stderr); + ABORT_NO_BACKTRACE(); +} +// Constant-initialized, so this is valid from load time, safe even for a +// CHECK() during early startup, before any SetAbortHandler call. +AbortHandler g_abort_handler = DefaultAbortHandler; +} // namespace + +void SetAbortHandler(AbortHandler handler) { + g_abort_handler = handler ? handler : DefaultAbortHandler; +} +AbortHandler GetAbortHandler() { + return g_abort_handler; +} + void Assert(const AssertionInfo& info) { std::string name = GetHumanReadableProcessName(); @@ -406,7 +430,7 @@ void Assert(const AssertionInfo& info) { info.message); fflush(stderr); - ABORT(); + ABORT_WITH_DETAILS(info.file_line, info.message); } enum class EnhanceFatalException { kEnhance, kDontEnhance }; @@ -584,7 +608,7 @@ static void ReportFatalException(Environment* env, } fflush(stderr); - ABORT(); + ABORT_WITH_DETAILS(location, message); } void OOMErrorHandler(const char* location, const v8::OOMDetails& details) { @@ -620,7 +644,7 @@ void OOMErrorHandler(const char* location, const v8::OOMDetails& details) { } fflush(stderr); - ABORT(); + ABORT_WITH_DETAILS(location, message); } v8::ModifyCodeGenerationFromStringsResult ModifyCodeGenerationFromStrings( diff --git a/src/node_ffi.cc b/src/node_ffi.cc index 1e1fc5654591..42c62c829168 100644 --- a/src/node_ffi.cc +++ b/src/node_ffi.cc @@ -1341,9 +1341,9 @@ static void Initialize(Local target, Boolean::New(isolate, CHAR_MIN < 0)) .Check(); - // The shared-buffer fast path uses `uintptrMax` to reject pointer BigInts - // that would otherwise be silently truncated by `ReadFFIArgFromBuffer`'s - // `memcpy(..., type->size, ...)` on 32-bit platforms. The slow path + // The JavaScript fast paths use `uintptrMax` to reject pointer BigInts that + // would otherwise be silently truncated by V8 or, on 32-bit platforms, by + // `ReadFFIArgFromBuffer`'s `memcpy(..., type->size, ...)`. The slow path // rejects the same values through `ToFFIArgument`. target ->Set(context, diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 68554ed33b31..9c80d18cdaab 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -1590,6 +1590,7 @@ void DatabaseSync::Prepare(const FunctionCallbackInfo& args) { Utf8Value sql(env->isolate(), args[0].As()); sqlite3_stmt* s = nullptr; int r = sqlite3_prepare_v2(db->connection_, *sql, -1, &s, nullptr); + StatementPtr stmt_ptr(s); CHECK_ERROR_OR_THROW(env->isolate(), db, r, SQLITE_OK, void()); @@ -1602,8 +1603,11 @@ void DatabaseSync::Prepare(const FunctionCallbackInfo& args) { return; } - BaseObjectPtr stmt = - StatementSync::Create(env, BaseObjectPtr(db), s); + BaseObjectPtr stmt = StatementSync::Create( + env, BaseObjectPtr(db), std::move(stmt_ptr)); + if (!stmt) { + return; + } db->statements_.insert(stmt.get()); if (return_arrays.has_value()) { @@ -2660,10 +2664,9 @@ int DatabaseSync::AuthorizerCallback(void* user_data, StatementSync::StatementSync(Environment* env, Local object, BaseObjectPtr db, - sqlite3_stmt* stmt) - : BaseObject(env, object), db_(std::move(db)) { + StatementPtr stmt) + : BaseObject(env, object), db_(std::move(db)), statement_(std::move(stmt)) { MakeWeak(); - statement_ = stmt; use_big_ints_ = db_->use_big_ints(); return_arrays_ = db_->return_arrays(); allow_bare_named_params_ = db_->allow_bare_named_params(); @@ -2677,15 +2680,15 @@ StatementSync::~StatementSync() { } void StatementSync::Close() { + db_->UntrackStatement(this); + if (!IsFinalized()) { - db_->UntrackStatement(this); Finalize(); } } void StatementSync::Finalize() { - sqlite3_finalize(statement_); - statement_ = nullptr; + statement_.reset(); InvalidateColumnNameCache(); } @@ -2714,11 +2717,11 @@ void StatementSync::Dispose(const FunctionCallbackInfo& args) { inline int StatementSync::ResetStatement() { reset_generation_++; - return sqlite3_reset(statement_); + return sqlite3_reset(statement_.get()); } bool StatementSync::BindParams(const FunctionCallbackInfo& args) { - int r = sqlite3_clear_bindings(statement_); + int r = sqlite3_clear_bindings(statement_.get()); CHECK_ERROR_OR_THROW(env()->isolate(), db_.get(), r, SQLITE_OK, false); int anon_idx = 1; @@ -2735,10 +2738,10 @@ bool StatementSync::BindParams(const FunctionCallbackInfo& args) { if (allow_bare_named_params_ && !bare_named_params_.has_value()) { bare_named_params_.emplace(); - int param_count = sqlite3_bind_parameter_count(statement_); + int param_count = sqlite3_bind_parameter_count(statement_.get()); // Parameter indexing starts at one. for (int i = 1; i <= param_count; ++i) { - const char* name = sqlite3_bind_parameter_name(statement_, i); + const char* name = sqlite3_bind_parameter_name(statement_.get(), i); if (name == nullptr) { continue; } @@ -2770,12 +2773,12 @@ bool StatementSync::BindParams(const FunctionCallbackInfo& args) { } Utf8Value utf8_key(env()->isolate(), key); - int r = sqlite3_bind_parameter_index(statement_, *utf8_key); + int r = sqlite3_bind_parameter_index(statement_.get(), *utf8_key); if (r == 0) { if (allow_bare_named_params_) { auto lookup = bare_named_params_->find(std::string(*utf8_key)); if (lookup != bare_named_params_->end()) { - r = sqlite3_bind_parameter_index(statement_, + r = sqlite3_bind_parameter_index(statement_.get(), lookup->second.c_str()); } } @@ -2805,7 +2808,8 @@ bool StatementSync::BindParams(const FunctionCallbackInfo& args) { for (int i = anon_start; i < args.Length(); ++i) { while (1) { - const char* param = sqlite3_bind_parameter_name(statement_, anon_idx); + const char* param = + sqlite3_bind_parameter_name(statement_.get(), anon_idx); if (param == nullptr || param[0] == '?') break; anon_idx++; } @@ -2831,7 +2835,7 @@ bool StatementSync::BindValue(const Local& value, const int index) { int r; if (value->IsNumber()) { const double val = value.As()->Value(); - r = sqlite3_bind_double(statement_, index, val); + r = sqlite3_bind_double(statement_.get(), index, val); } else if (value->IsString()) { Utf8Value val(isolate, value.As()); if (val.IsAllocated()) { @@ -2841,9 +2845,9 @@ bool StatementSync::BindValue(const Local& value, const int index) { const sqlite3_uint64 length = static_cast(val.length()); val.Release(); r = sqlite3_bind_text64( - statement_, index, data, length, std::free, SQLITE_UTF8); + statement_.get(), index, data, length, std::free, SQLITE_UTF8); } else { - r = sqlite3_bind_text64(statement_, + r = sqlite3_bind_text64(statement_.get(), index, *val, static_cast(val.length()), @@ -2851,17 +2855,17 @@ bool StatementSync::BindValue(const Local& value, const int index) { SQLITE_UTF8); } } else if (value->IsNull()) { - r = sqlite3_bind_null(statement_, index); + r = sqlite3_bind_null(statement_.get(), index); } else if (value->IsArrayBufferView() || value->IsArrayBuffer() || value->IsSharedArrayBuffer()) { ArrayBufferViewContents buf(value); - r = sqlite3_bind_blob64(statement_, + r = sqlite3_bind_blob64(statement_.get(), index, buf.data(), static_cast(buf.length()), SQLITE_TRANSIENT); } else if (value->IsBoolean()) { - r = sqlite3_bind_int(statement_, index, value->IsTrue() ? 1 : 0); + r = sqlite3_bind_int(statement_.get(), index, value->IsTrue() ? 1 : 0); } else if (value->IsBigInt()) { bool lossless; int64_t as_int = value.As()->Int64Value(&lossless); @@ -2869,7 +2873,7 @@ bool StatementSync::BindValue(const Local& value, const int index) { THROW_ERR_INVALID_ARG_VALUE(env(), "BigInt value is too large to bind."); return false; } - r = sqlite3_bind_int64(statement_, index, as_int); + r = sqlite3_bind_int64(statement_.get(), index, as_int); } else { THROW_ERR_INVALID_ARG_TYPE( isolate, @@ -2884,11 +2888,11 @@ bool StatementSync::BindValue(const Local& value, const int index) { MaybeLocal StatementSync::ColumnToValue(const int column) { return StatementExecutionHelper::ColumnToValue( - env(), statement_, column, use_big_ints_); + env(), statement_.get(), column, use_big_ints_); } MaybeLocal StatementSync::ColumnNameToName(const int column) { - const char* col_name = sqlite3_column_name(statement_, column); + const char* col_name = sqlite3_column_name(statement_.get(), column); if (col_name == nullptr) { THROW_ERR_INVALID_STATE(env(), "Cannot get name of column %d", column); return MaybeLocal(); @@ -2905,10 +2909,10 @@ bool StatementSync::GetCachedColumnNames(LocalVector* keys) { Isolate* isolate = env()->isolate(); const int reprepare_count = - sqlite3_stmt_status(statement_, SQLITE_STMTSTATUS_REPREPARE, false); + sqlite3_stmt_status(statement_.get(), SQLITE_STMTSTATUS_REPREPARE, false); if (reprepare_count != cached_column_names_reprepare_count_) { cached_column_names_.clear(); - const int num_cols = sqlite3_column_count(statement_); + const int num_cols = sqlite3_column_count(statement_.get()); if (num_cols == 0) { cached_column_names_reprepare_count_ = reprepare_count; return true; @@ -3184,17 +3188,17 @@ void StatementSync::All(const FunctionCallbackInfo& args) { bool needs_reset = true; auto reset = OnScopeLeave([&]() { - if (needs_reset) sqlite3_reset(stmt->statement_); + if (needs_reset) sqlite3_reset(stmt->statement_.get()); }); Local result; if (StatementExecutionHelper::All(env, stmt->db_.get(), - stmt->statement_, + stmt->statement_.get(), stmt->return_arrays_, stmt->use_big_ints_) .ToLocal(&result)) { RESET_AND_CHECK( - isolate, stmt->db_.get(), stmt->statement_, needs_reset, void()); + isolate, stmt->db_.get(), stmt->statement_.get(), needs_reset, void()); args.GetReturnValue().Set(result); } } @@ -3238,7 +3242,7 @@ void StatementSync::Get(const FunctionCallbackInfo& args) { Local result; if (StatementExecutionHelper::Get(env, stmt->db_.get(), - stmt->statement_, + stmt->statement_.get(), stmt->return_arrays_, stmt->use_big_ints_) .ToLocal(&result)) { @@ -3261,7 +3265,7 @@ void StatementSync::Run(const FunctionCallbackInfo& args) { Local result; if (StatementExecutionHelper::Run( - env, stmt->db_.get(), stmt->statement_, stmt->use_big_ints_) + env, stmt->db_.get(), stmt->statement_.get(), stmt->use_big_ints_) .ToLocal(&result)) { args.GetReturnValue().Set(result); } @@ -3273,7 +3277,7 @@ void StatementSync::Columns(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); THROW_AND_RETURN_ON_BAD_STATE( env, stmt->IsFinalized(), "statement has been finalized"); - int num_cols = sqlite3_column_count(stmt->statement_); + int num_cols = sqlite3_column_count(stmt->statement_.get()); Isolate* isolate = env->isolate(); LocalVector cols(isolate); auto sqlite_column_template = env->sqlite_column_template(); @@ -3288,14 +3292,14 @@ void StatementSync::Columns(const FunctionCallbackInfo& args) { for (int i = 0; i < num_cols; ++i) { MaybeLocal values[] = { NullableSQLiteStringToValue( - isolate, sqlite3_column_origin_name(stmt->statement_, i)), + isolate, sqlite3_column_origin_name(stmt->statement_.get(), i)), NullableSQLiteStringToValue( - isolate, sqlite3_column_database_name(stmt->statement_, i)), + isolate, sqlite3_column_database_name(stmt->statement_.get(), i)), stmt->ColumnNameToName(i), NullableSQLiteStringToValue( - isolate, sqlite3_column_table_name(stmt->statement_, i)), + isolate, sqlite3_column_table_name(stmt->statement_.get(), i)), NullableSQLiteStringToValue( - isolate, sqlite3_column_decltype(stmt->statement_, i)), + isolate, sqlite3_column_decltype(stmt->statement_.get(), i)), }; Local col; @@ -3317,7 +3321,7 @@ void StatementSync::SourceSQLGetter(const FunctionCallbackInfo& args) { THROW_AND_RETURN_ON_BAD_STATE( env, stmt->IsFinalized(), "statement has been finalized"); Local sql; - if (!String::NewFromUtf8(env->isolate(), sqlite3_sql(stmt->statement_)) + if (!String::NewFromUtf8(env->isolate(), sqlite3_sql(stmt->statement_.get())) .ToLocal(&sql)) { return; } @@ -3332,7 +3336,7 @@ void StatementSync::ExpandedSQLGetter(const FunctionCallbackInfo& args) { env, stmt->IsFinalized(), "statement has been finalized"); // sqlite3_expanded_sql may return nullptr without producing an error code. - char* expanded = sqlite3_expanded_sql(stmt->statement_); + char* expanded = sqlite3_expanded_sql(stmt->statement_.get()); if (expanded == nullptr) { return THROW_ERR_SQLITE_ERROR( env->isolate(), "Expanded SQL text would exceed configured limits"); @@ -3506,11 +3510,11 @@ bool SQLTagStore::ResetAndBindStatement( int r = stmt->ResetStatement(); CHECK_ERROR_OR_THROW(isolate, stmt->db_.get(), r, SQLITE_OK, false); - r = sqlite3_clear_bindings(stmt->statement_); + r = sqlite3_clear_bindings(stmt->statement_.get()); CHECK_ERROR_OR_THROW(isolate, stmt->db_.get(), r, SQLITE_OK, false); uint32_t n_params = args.Length() - 1; - int param_count = sqlite3_bind_parameter_count(stmt->statement_); + int param_count = sqlite3_bind_parameter_count(stmt->statement_.get()); if (param_count != static_cast(n_params)) { THROW_ERR_INVALID_ARG_VALUE( env, @@ -3547,7 +3551,7 @@ void SQLTagStore::Run(const FunctionCallbackInfo& args) { Local result; if (StatementExecutionHelper::Run( - env, stmt->db_.get(), stmt->statement_, stmt->use_big_ints_) + env, stmt->db_.get(), stmt->statement_.get(), stmt->use_big_ints_) .ToLocal(&result)) { args.GetReturnValue().Set(result); } @@ -3602,7 +3606,7 @@ void SQLTagStore::Get(const FunctionCallbackInfo& args) { Local result; if (StatementExecutionHelper::Get(env, stmt->db_.get(), - stmt->statement_, + stmt->statement_.get(), stmt->return_arrays_, stmt->use_big_ints_) .ToLocal(&result)) { @@ -3631,17 +3635,17 @@ void SQLTagStore::All(const FunctionCallbackInfo& args) { Isolate* isolate = env->isolate(); bool needs_reset = true; auto reset = OnScopeLeave([&]() { - if (needs_reset) sqlite3_reset(stmt->statement_); + if (needs_reset) sqlite3_reset(stmt->statement_.get()); }); Local result; if (StatementExecutionHelper::All(env, stmt->db_.get(), - stmt->statement_, + stmt->statement_.get(), stmt->return_arrays_, stmt->use_big_ints_) .ToLocal(&result)) { RESET_AND_CHECK( - isolate, stmt->db_.get(), stmt->statement_, needs_reset, void()); + isolate, stmt->db_.get(), stmt->statement_.get(), needs_reset, void()); args.GetReturnValue().Set(result); } } @@ -3704,10 +3708,10 @@ BaseObjectPtr SQLTagStore::PrepareStatement( sqlite3_stmt* s = nullptr; int r = sqlite3_prepare_v2( session->database_->connection_, sql.data(), sql.size(), &s, nullptr); + StatementPtr stmt_ptr(s); if (r != SQLITE_OK) { THROW_ERR_SQLITE_ERROR(isolate, session->database_.get()); - sqlite3_finalize(s); return BaseObjectPtr(); } @@ -3718,15 +3722,17 @@ BaseObjectPtr SQLTagStore::PrepareStatement( return BaseObjectPtr(); } - BaseObjectPtr stmt_obj = StatementSync::Create( - env, BaseObjectPtr(session->database_), s); + BaseObjectPtr stmt_obj = + StatementSync::Create(env, + BaseObjectPtr(session->database_), + std::move(stmt_ptr)); if (!stmt_obj) { THROW_ERR_SQLITE_ERROR(isolate, "Failed to create StatementSync"); - sqlite3_finalize(s); return BaseObjectPtr(); } + session->database_->statements_.insert(stmt_obj.get()); session->sql_tags_.Put(sql, stmt_obj); stmt = stmt_obj; } @@ -3789,7 +3795,7 @@ Local StatementSync::GetConstructorTemplate( } BaseObjectPtr StatementSync::Create( - Environment* env, BaseObjectPtr db, sqlite3_stmt* stmt) { + Environment* env, BaseObjectPtr db, StatementPtr stmt) { Local obj; if (!GetConstructorTemplate(env) ->InstanceTemplate() @@ -3798,7 +3804,8 @@ BaseObjectPtr StatementSync::Create( return nullptr; } - return MakeBaseObject(env, obj, std::move(db), stmt); + return MakeBaseObject( + env, obj, std::move(db), std::move(stmt)); } StatementSyncIterator::StatementSyncIterator(Environment* env, @@ -3871,14 +3878,14 @@ void StatementSyncIterator::Next(const FunctionCallbackInfo& args) { iter->statement_reset_generation_ != iter->stmt_->reset_generation_, "iterator was invalidated"); - int r = sqlite3_step(iter->stmt_->statement_); + int r = sqlite3_step(iter->stmt_->statement_.get()); if (r != SQLITE_ROW) { CHECK_ERROR_OR_THROW( env->isolate(), iter->stmt_->db_.get(), r, SQLITE_DONE, void()); iter->done_ = true; RESET_OR_THROW(env->isolate(), iter->stmt_->db_.get(), - iter->stmt_->statement_, + iter->stmt_->statement_.get(), void()); MaybeLocal values[] = {Boolean::New(isolate, true), Null(isolate)}; Local result; @@ -3889,13 +3896,13 @@ void StatementSyncIterator::Next(const FunctionCallbackInfo& args) { return; } - int num_cols = sqlite3_column_count(iter->stmt_->statement_); + int num_cols = sqlite3_column_count(iter->stmt_->statement_.get()); Local row_value; LocalVector row_keys(isolate); LocalVector row_values(isolate); if (ExtractRowValues(env, - iter->stmt_->statement_, + iter->stmt_->statement_.get(), num_cols, iter->stmt_->use_big_ints_, &row_values) @@ -3935,7 +3942,7 @@ void StatementSyncIterator::Return(const FunctionCallbackInfo& args) { // is invoked by the language during abrupt completion (e.g. a `throw` // inside a `for...of` body), and throwing on a deferred SQLite error // would discard the caller's already-pending exception. - sqlite3_reset(iter->stmt_->statement_); + sqlite3_reset(iter->stmt_->statement_.get()); iter->done_ = true; auto iter_template = getLazyIterTemplate(env); diff --git a/src/node_sqlite.h b/src/node_sqlite.h index b4446e5db859..17025a528622 100644 --- a/src/node_sqlite.h +++ b/src/node_sqlite.h @@ -135,6 +135,12 @@ class StatementSync; class BackupJob; class Session; +inline void FinalizeStatement(sqlite3_stmt* stmt) { + sqlite3_finalize(stmt); +} + +using StatementPtr = DeleteFnPtr; + class StatementExecutionHelper { public: static v8::MaybeLocal All(Environment* env, @@ -263,13 +269,13 @@ class StatementSync : public BaseObject { StatementSync(Environment* env, v8::Local object, BaseObjectPtr db, - sqlite3_stmt* stmt); + StatementPtr stmt); void MemoryInfo(MemoryTracker* tracker) const override; static v8::Local GetConstructorTemplate( Environment* env); static BaseObjectPtr Create(Environment* env, BaseObjectPtr db, - sqlite3_stmt* stmt); + StatementPtr stmt); static void All(const v8::FunctionCallbackInfo& args); static void Iterate(const v8::FunctionCallbackInfo& args); static void Get(const v8::FunctionCallbackInfo& args); @@ -299,7 +305,7 @@ class StatementSync : public BaseObject { ~StatementSync() override; void Close(); BaseObjectPtr db_; - sqlite3_stmt* statement_; + StatementPtr statement_; bool return_arrays_ = false; bool use_big_ints_; bool allow_bare_named_params_; diff --git a/src/node_version.h b/src/node_version.h index 4c18037fc8c8..8c94ce8ec7f3 100644 --- a/src/node_version.h +++ b/src/node_version.h @@ -31,6 +31,10 @@ #define NODE_VERSION_IS_RELEASE 0 +#define NODE_ALPHA_MAJOR_VERSION 0 +#define NODE_ALPHA_MINOR_VERSION 0 +#define NODE_ALPHA_PATCH_VERSION 0 + #ifndef NODE_STRINGIFY #define NODE_STRINGIFY(n) NODE_STRINGIFY_HELPER(n) #define NODE_STRINGIFY_HELPER(n) #n @@ -41,18 +45,24 @@ #endif #ifndef NODE_TAG -# if NODE_VERSION_IS_RELEASE -# define NODE_TAG "" -# else -# define NODE_TAG "-pre" -# endif +#if NODE_VERSION_IS_RELEASE +#ifdef NODE_ALPHA_MAJOR_VERSION +#define NODE_TAG \ + "-alpha." NODE_STRINGIFY(NODE_ALPHA_MAJOR_VERSION) "." NODE_STRINGIFY( \ + NODE_ALPHA_MINOR_VERSION) "." NODE_STRINGIFY(NODE_ALPHA_PATCH_VERSION) #else +#define NODE_TAG "" +#endif // NODE_ALPHA_MAJOR_VERSION +#else // NODE_VERSION_IS_RELEASE +#define NODE_TAG "-pre" +#endif // NODE_VERSION_IS_RELEASE +#else // NODE_TAG // NODE_TAG is passed without quotes when rc.exe is run from msbuild # define NODE_EXE_VERSION NODE_STRINGIFY(NODE_MAJOR_VERSION) "." \ NODE_STRINGIFY(NODE_MINOR_VERSION) "." \ NODE_STRINGIFY(NODE_PATCH_VERSION) \ NODE_STRINGIFY(NODE_TAG) -#endif +#endif // NODE_TAG # define NODE_VERSION_STRING NODE_STRINGIFY(NODE_MAJOR_VERSION) "." \ NODE_STRINGIFY(NODE_MINOR_VERSION) "." \ diff --git a/src/util.h b/src/util.h index a979f626bdb4..5e9bf2b1cdab 100644 --- a/src/util.h +++ b/src/util.h @@ -128,6 +128,9 @@ void NODE_EXTERN_PRIVATE Assert(const AssertionInfo& info); void DumpNativeBacktrace(FILE* fp); void DumpJavaScriptBacktrace(FILE* fp); +// Returns the currently installed abort handler which is never null. +AbortHandler GetAbortHandler(); + // Windows 8+ does not like abort() in Release mode #ifdef _WIN32 #define ABORT_NO_BACKTRACE() _exit(static_cast(node::ExitCode::kAbort)) @@ -140,13 +143,12 @@ void DumpJavaScriptBacktrace(FILE* fp); // when generating code for them the compiler can choose not to // maintain the frame pointers or link registers that are necessary for // correct backtracing. -// `ABORT` must be a macro and not a [[noreturn]] function to make sure the -// backtrace is correct. -#define ABORT() \ +// `ABORT` and `ABORT_WITH_DETAILS` must be a macro and not a [[noreturn]] +// function to make sure the backtrace is correct. +#define ABORT() ABORT_WITH_DETAILS(__FILE__ ":" STRINGIFY(__LINE__), nullptr) +#define ABORT_WITH_DETAILS(location, message) \ do { \ - node::DumpNativeBacktrace(stderr); \ - node::DumpJavaScriptBacktrace(stderr); \ - fflush(stderr); \ + node::GetAbortHandler()(location, message); \ ABORT_NO_BACKTRACE(); \ } while (0) diff --git a/test/addons/abort-handler/binding.cc b/test/addons/abort-handler/binding.cc new file mode 100644 index 000000000000..13ceb011e57b --- /dev/null +++ b/test/addons/abort-handler/binding.cc @@ -0,0 +1,18 @@ +#include +#include +#include + +namespace { +void TestAbortHandler(const char* /*location*/, const char* /*message*/) { + fputs("CUSTOM_ABORT_HANDLER_RAN\n", stderr); + fflush(stderr); +} + +void InstallAbortHandler(const v8::FunctionCallbackInfo&) { + node::SetAbortHandler(TestAbortHandler); +} +} // namespace + +NODE_MODULE_INIT() { + NODE_SET_METHOD(exports, "installAbortHandler", InstallAbortHandler); +} diff --git a/test/addons/abort-handler/binding.gyp b/test/addons/abort-handler/binding.gyp new file mode 100644 index 000000000000..55fbe7050f18 --- /dev/null +++ b/test/addons/abort-handler/binding.gyp @@ -0,0 +1,9 @@ +{ + 'targets': [ + { + 'target_name': 'binding', + 'sources': [ 'binding.cc' ], + 'includes': ['../common.gypi'], + } + ] +} diff --git a/test/addons/abort-handler/test.js b/test/addons/abort-handler/test.js new file mode 100644 index 000000000000..4c05046c92b1 --- /dev/null +++ b/test/addons/abort-handler/test.js @@ -0,0 +1,45 @@ +'use strict'; +const common = require('../../common'); +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); +const { exec } = require('child_process'); + +const bindingPath = path.resolve( + __dirname, 'build', common.buildType, 'binding.node'); + +if (!fs.existsSync(bindingPath)) + common.skip('binding not built yet'); + +if (process.argv[2] === 'child') { + const binding = require(bindingPath); + binding.installAbortHandler(); + process.abort(); + return; +} + +const escapedArgs = + common.escapePOSIXShell`"${process.execPath}" "${__filename}" child`; +if (!common.isWindows) { + // Do not create core files, as it can take a lot of disk space on + // continuous testing and developers' machines. + escapedArgs[0] = 'ulimit -c 0 && ' + escapedArgs[0]; +} + +exec(...escapedArgs, common.mustCall((err, stdout, stderr) => { + assert.ok( + stderr.includes('CUSTOM_ABORT_HANDLER_RAN'), + `Expected custom abort handler marker in stderr, got:\n${stderr}`); + assert.ok( + !stderr.includes('Native stack trace'), + `Expected the custom handler to replace the default dump, got:\n${stderr}`); + + // The child aborts. Whether that surfaces as the SIGABRT signal or as exit + // code 134 depends on shell wrapping: the `ulimit -c 0 && ...` prefix makes + // /bin/sh wait on (rather than exec-replace itself with) the node grandchild, + // so sh reports the aborted grandchild as a normal exit with code 134. + // common.nodeProcessAborted() accepts both forms. + assert.ok( + err && common.nodeProcessAborted(err.code, err.signal), + `Expected the child to abort, got code=${err?.code} signal=${err?.signal}`); +})); diff --git a/test/cctest/test_environment.cc b/test/cctest/test_environment.cc index 59c71835499e..fb1bcc2ef90c 100644 --- a/test/cctest/test_environment.cc +++ b/test/cctest/test_environment.cc @@ -1201,3 +1201,57 @@ TEST_F(EnvironmentTest, LoadEnvironmentWithCallbackWithESModule) { printf("Frame: %s\n", *frame_str); EXPECT_EQ(frame_str.ToString(), " at embedded:esm.mjs:3:15"); } + +namespace { +void CustomAbortHandlerForContractTest(const char* location, + const char* message) {} + +bool abort_handler_dispatch_flag = false; +const char* abort_handler_received_location = nullptr; +const char* abort_handler_received_message = nullptr; +void AbortHandlerThatSetsDispatchFlag(const char* location, + const char* message) { + abort_handler_dispatch_flag = true; + abort_handler_received_location = location; + abort_handler_received_message = message; +} +} // namespace + +TEST(AbortHandlerTest, DefaultIsNonNullAndSetAbortHandlerRoundTrips) { + node::AbortHandler old = node::GetAbortHandler(); + + // There should always be a non-null default handler installed. + EXPECT_NE(node::GetAbortHandler(), nullptr); + + node::SetAbortHandler(CustomAbortHandlerForContractTest); + EXPECT_EQ(node::GetAbortHandler(), CustomAbortHandlerForContractTest); + + node::SetAbortHandler(nullptr); + EXPECT_NE(node::GetAbortHandler(), nullptr); + EXPECT_NE(node::GetAbortHandler(), CustomAbortHandlerForContractTest); + + node::SetAbortHandler(old); +} + +TEST(AbortHandlerTest, InstalledHandlerIsInvokedWhenCalled) { + node::AbortHandler old = node::GetAbortHandler(); + abort_handler_dispatch_flag = false; + abort_handler_received_location = nullptr; + abort_handler_received_message = nullptr; + + node::SetAbortHandler(AbortHandlerThatSetsDispatchFlag); + node::AbortHandler h = node::GetAbortHandler(); + // Fail cleanly (instead of crashing on a null call) if the handler wasn't + // actually installed. + ASSERT_NE(h, nullptr); + + // Dispatch through the public GetAbortHandler() accessor directly (not via + // the ABORT() macro, so nothing terminates), and verify the message is + // passed through unchanged. + node::GetAbortHandler()("some-test-location", "some-test-message"); + EXPECT_TRUE(abort_handler_dispatch_flag); + EXPECT_STREQ(abort_handler_received_location, "some-test-location"); + EXPECT_STREQ(abort_handler_received_message, "some-test-message"); + + node::SetAbortHandler(old); +} diff --git a/test/ffi/test-ffi-fast-integer-validation.js b/test/ffi/test-ffi-fast-integer-validation.js index 26d51ae4248f..1243391ac87c 100644 --- a/test/ffi/test-ffi-fast-integer-validation.js +++ b/test/ffi/test-ffi-fast-integer-validation.js @@ -68,3 +68,34 @@ test('fast FFI validates integer argument ranges', () => { lib.close(); } }); + +test('fast FFI validates pointer BigInt ranges', () => { + const lib = new ffi.DynamicLibrary(libraryPath); + try { + for (const type of ['pointer', 'ptr', 'string', 'str', + 'buffer', 'arraybuffer']) { + const identityPointer = lib.getFunction('identity_pointer', { + arguments: [type], + return: 'pointer', + }); + const sumBuffer = lib.getFunction('sum_buffer', { + arguments: [type, 'u64'], + return: 'u64', + }); + function callSingle(value) { return identityPointer(value); } + + function callMultiple(value) { return sumBuffer(value, 0n); } + + optimize(callSingle, 0n); + optimize(callMultiple, 0n); + + const expect = { code: 'ERR_INVALID_ARG_VALUE' }; + for (const call of [callSingle, callMultiple]) { + assert.throws(() => call(-1n), expect); + assert.throws(() => call((2n ** 64n) + 5n), expect); + } + } + } finally { + lib.close(); + } +}); diff --git a/test/parallel/test-sqlite-template-tag.js b/test/parallel/test-sqlite-template-tag.js index 20376e199d1b..eaa6d19fc7cd 100644 --- a/test/parallel/test-sqlite-template-tag.js +++ b/test/parallel/test-sqlite-template-tag.js @@ -371,3 +371,20 @@ test('tag store prevents circular reference leaks', async () => { return after < before * 1.5; }, 20); }); + +test('cached statements are finalized when the database is closed', () => { + const db = new DatabaseSync(':memory:'); + const sql = db.createTagStore(); + + db.exec('CREATE TABLE foo (id INTEGER PRIMARY KEY)'); + db.exec('INSERT INTO foo (id) VALUES (1)'); + assert.deepStrictEqual(sql.all`SELECT id FROM foo`, [{ __proto__: null, id: 1 }]); + + db.close(); + db.open(); + + assert.throws(() => sql.all`SELECT id FROM foo`, { + code: 'ERR_SQLITE_ERROR', + message: /no such table/i, + }); +});