Skip to content

[v4.x] deps: revert/re-apply 09db540 from upstream v8

This PR aims to fix an improperly applied backport to the v8 deps on the v4.x series.

As discussed and discovered in https://github.com/nodejs/node/issues/14228, the original intention of https://github.com/nodejs/node/commit/3e8d7a73aa5d17f070ab17741e48249ea5a9853c was to apply https://github.com/v8/v8/commit/e093a04 and https://github.com/v8/v8/commit/09db540 but that backport commit failed to consider that the former commit (e093) was reverted in https://github.com/v8/v8/commit/5f5a3282d4b21eb0800f1d1af85926670ba0d904. This led to inadvertent changes (which were never present on v8) to MarkCompactCollector::ClearWeakCollections being left in place on the Node.js 4.x series, in addition to the (incorrect) changes to the weakset and weakmap tests.

That MarkCompactCollector change is directly responsible for some errors, specifically noted in the stack traces of the post-mortems reported by myself and others in https://github.com/nodejs/node/issues/14228 and https://github.com/meteor/meteor/pull/8970#issuecomment-319877857.

Furthermore, while the correct fix to v8 was meant to apply the "Wipe deleted entries" logic to HashTable::Rehash it was also applied to CompilationCacheTable::Age as well. You'll note that an additional, seemingly arbitrary capacity variable needed to be declared in the backport which was not present in the original patch. Not too surprising as the patch applies cleanly to that class as well, albeit with a several thousand line patch offset. (This caught me off guard during debugging as well and it looks as if the v8 team was also bit by this temporarily in https://github.com/v8/v8/commit/59c7657d642bf500c49da3c4bd0e6d7bf647ec7d!)

In this process, I also took the liberty to backport the very relevant https://github.com/v8/v8/commit/686558d which corrects the comment on the original commit.

It's my belief that this now matches up with the intended fix for https://github.com/nodejs/node/issues/6180 and crbug.com/v8/4909 and also maintains relevant follow-ups, such as https://github.com/v8/v8/commit/b93c80a6 and https://github.com/v8/v8/commit/a76d133f.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

V8 Engine

This is my first v8 commit, so happy to adjust accordingly, but this is what I've found!

Merge request reports

Loading