Update V8 to 13.0.245.25 #113
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- adapts library names of third party libs (certain libs like zlib are now prefixed by third_party_) - removes static ctor of JavascriptContext - after unmanaged init engine flags cannot be set - move unmanaged init to normal ctor so users can call JavascriptContext.SetFlags - throw exception in SetFlags method if unmanaged init has already happened - alter flag test - since we can only set flags once remove the --strict flag test as it would require a lot of test changes - set --stack-size in a global test init method instead of --strict which also improves test reliability when run in a command line setting (e.g. build server) - add test that verifies an exception is thrown when flags are set after initialization - adds new property to get a process global boolean if the unmanaged initialization of V8 has already been done Certain operations like setting engine flags are only allowed before the V8 platform is initialized. Since managed C# code does not have support for real process global state (static globals are always local to an app domain), you can't really determine and persist the initialization state in calling code. The new property passes through the unmanaged real process global initialization state of V8 instead to mitigate that.
iterator, schedule them as proper JS exceptions, and return an IteratorResult object indicating the end of the iterator. If we don't catch and schedule here the .NET exception leads to an immediate termination of the script execution with no way of catching the error in JS land. This can be especially bad if the JS code does manual resource management (e.g. closing a file handle) in a finally block that is then never executed. We need to further ensure that the exception detail is filled correctly if an exception occurs during iteration. The JS error object we create and schedule in case of a .NET exception during the next() method of the iterator, must also be set as the return value of the next() method. Only this way the exception detail is set correctly (including source and line information).
- C++ Standard -> 20 which is required by the V8 headers - add the /Zc:__cplusplus directive to report the C++ version correctly to the preprocessor otherwise the V8 headers complain with an #error directive (see https://learn.microsoft.com/en-us/cpp/build/reference/zc-cplusplus?view=msvc-170) - add /DNOMINMAX directive to add a #define NOMINMAX otherwise std::numeric_limits<int32_t>::max() calls in the V8 header fail because they somehow get replaced by a min/max call from some stray Win32 header - use the correct signature and semantics of the new getter/setter interceptor callbacks - address warnings regarding `Holder()` as reported and hinted by V8
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Since there is some traction again I thought I'd contribute all the V8 updates and one fix we have done during that work in our fork. The V8 breaking changes unfortunately are plenty again, so I wanted to open this early so you are aware and can plan accordingly. I'm happy to wait until the .NET 8 migration is done and then rebasing/merging this PR on top of that, so no rush there or here. This PR replaces #102 which I will close. Let me know how you want to proceed.
This contains 3 updates to V8 and a fix to enable proper exception handling in JS when an iterator (i.e. the
MoveNextmethod) from .NET land fails. Each update and fix is its own commit with a detailed message. Here's a summary.9.8 -> 10.0
Nothing much happened, just an extra isolate parameter that was actually removed again in 13.0 ...
10.0 -> 11.9
This unfortunately not only has breaking-changes in the API but also the distributables and especially the behavior of setting flags.
Most of the third party libraries of V8 are now prefixed by
third_party_so the post build scripts were adapted to copy the correct DLLs over.Setting engine flags like e.g.
--stack_sizeor--use-strictis not allowed anymore onceV8::InitializePlatformis done inUnmanagedInitialization. This means the unmanaged init must not happen anymore in a static ctor for .NET because otherwise you can never useSetFlags. Instead the init is moved to the normal ctor (it was thread-safe already) and in addition a)SetFlagis now guarded and throws an exception if V8 was already initialized, and b) we need a new property for .NET land to query if the init has already happened (IsV8Initialized).Fix for exception during
MoveNextcallsWith the iterator implementation as-is an exception during the
MoveNextcall leads to an immediate termination of the script and you're not able to catch said error in JS land. This can be bad if you usetry .. finallyblocks in JS to do resource management (e.g. close a file handle). To properly do that we can schedule the .NET exception as a normal JS exception with the script and follow the correct JS iterator protocol.11.9 -> 13.0
The usual breaking-changes in the V8 API especially regarding the getter/setter interceptor callbacks. Those now need to state explicitly if the call was intercepted correctly and some names of callback types have changed. V8 reported incorrect usages of the deprecated
Holder()calls that were replaced by eitherThis()orHolderV2().In addition to that the V8 headers now require C++ standard 20 so it is set accordingly. For that to work properly, however, we need to also add the
/Zc:__cplusplusdirective to the compiler flags. Otherwise the preprocessor directives reporting the C++ version return the wrong version and the V8 headers complain with an #error directive (see https://learn.microsoft.com/en-us/cpp/build/reference/zc-cplusplus?view=msvc-170). And finally there are conflicts with calls tostd::numeric_limits<int32_t>::max()in the V8 header and some Win32 header which is mitigated by adding the /DNOMINMAX define.