node icon indicating copy to clipboard operation
node copied to clipboard

Update V8 before the release of v19.0.0

Open targos opened this issue 1 year ago • 13 comments

Currently, V8 on main is at version 10.2.154.15 (the same as v18.x). The stable V8 version today is 10.5 and 10.7 will be stable on Oct 18 (the release date of Node.js 19.0.0).

Unfortunately, a few issues have been preventing us from upgrading for months and they are still unresolved. Keeping V8 up to date is essential to Node.js' success in the long term, so it would be very unfortunate to release v19.0.0 without a newer version.

Here are the issues blocking the upgrade:

CI is still running with the latest canary (https://ci.nodejs.org/job/node-test-commit/56400/). I hope it won't find other issues.

/cc @nodejs/collaborators @nodejs/tsc @nodejs/v8 any help will be appreciated.

targos avatar Sep 15 '22 09:09 targos

These are all C/C++ related issues, right? (If so, sorry, I haven't used that in ~10-15 years 😞)

JakobJingleheimer avatar Sep 15 '22 09:09 JakobJingleheimer

Only "Build error Windows" seems like a showstopper, that's a tier 1 platform. I'll take a look.

The other platforms are not tier 1. It's okay to go ahead and release v19.0.0 without binaries for them and fix it later.

W.r.t. https://github.com/nodejs/node-v8/issues/236: https://github.com/nodejs/node/pull/44049 - just needs minor fix-ups for Windows, IIRC.

bnoordhuis avatar Sep 15 '22 10:09 bnoordhuis

I guess the "crashes with debug build" are also relevant for release binaries as these checks are an indication that something wrong is done.

Flarna avatar Sep 15 '22 10:09 Flarna

They are, but I don't think it's really serious and something we can fix over time.

The problem is well-known (calling into the runtime when it's tearing down) and there's a sledgehammer fix if necessary:

diff --git a/src/env-inl.h b/src/env-inl.h
index e04c0c2cf17..49c16340a67 100644
--- a/src/env-inl.h
+++ b/src/env-inl.h
@@ -607,7 +607,9 @@ void Environment::RequestInterrupt(Fn&& cb) {
 }
 
 inline bool Environment::can_call_into_js() const {
-  return can_call_into_js_ && !is_stopping();
+  return can_call_into_js_ &&
+         !is_stopping() &&
+         !isolate_->IsExecutionTerminating();
 }
 
 inline void Environment::set_can_call_into_js(bool can_call_into_js) {

bnoordhuis avatar Sep 15 '22 10:09 bnoordhuis

10.7 will be stable on Oct 18 (the release date of Node.js 19.0.0).

Question for curiosity, absolutely not blocking: is this potentially worth shifting 19.0.0 back a week for?

bnb avatar Sep 15 '22 13:09 bnb

I'd go one step further: we should wait releasing v19 until this is sorted.

mcollina avatar Sep 15 '22 13:09 mcollina

@mcollina I'm not sure about dealying v19. If we can't resolve in the next month then it could be months more before we do. Since v19 is not going to be promoted to LTS I think its a bit less crucial to have the latest LTS.

I do agree that keeping V8 up to date is important but delaying v19 for a long time would have consequences for our LTS release, as by policy LTS has to happen after the new current, and also by policy things need to bake in Current, so Node.js 18 would stagnate. We've worked quite hard to make our releases predictable and to stick to the target dates.

mhdawson avatar Sep 15 '22 15:09 mhdawson

And to clarify, I'm not necessary concerned about a 1 week slip to give us a week, but delaying indefintely is a concern.

mhdawson avatar Sep 15 '22 15:09 mhdawson

Hi folks.

Since v19 is not going to be promoted to LTS I think its a bit less crucial to have the latest LTS

if v19 is not LTS would it be possible to bring the v8 bump in a later 19.x.x ? (a week later) then there would at least give the new v8 time to bake.

srl295 avatar Sep 15 '22 15:09 srl295

if v19 is not LTS would it be possible to bring the v8 bump in a later 19.x.x ? (a week later) then there would at least give the new v8 time to bake.

Current versions of Node.js are subject to semver as much as LTS ones, if upgrading V8 is a breaking change (and it likely will be), it won't be able to land. There are of course ways to workaround this (e.g. by applying patches on top of V8 to address the breaking changes), and the TSC can also decide to disregard semver rules and land it even if it's breaking (there's precedent for it), but it would be better for everyone to have an up-to-date version of V8 already on main by the time we release v19.0.0.

Maybe discussions regarding the release schedule should be taken to nodejs/Release, and we should keep this issue focus on fixing the V8 issues.

aduh95 avatar Sep 15 '22 16:09 aduh95

if v19 is not LTS would it be possible to bring the v8 bump in a later 19.x.x ? (a week later) then there would at least give the new v8 time to bake.

IMO no we should not do mid-line upgrade, it'd need to go into 20 (which would be 6 months away).

bnb avatar Sep 15 '22 23:09 bnb

@bahamat can you help resolve the smartos issue?

mhdawson avatar Sep 16 '22 13:09 mhdawson

@mhdawson Yes, I’ll see what we can do.

bahamat avatar Sep 16 '22 13:09 bahamat