Skip to content

Commit

Permalink
Refactor jsg::V8StackScope into callback-based API
Browse files Browse the repository at this point in the history
Defining `jsg::V8StackScope` as a stack is problematic as it does
not guarantee that the stack start will be set correctly in v8.
This commit refactors the API to use a callback pattern instead.

```
jsg::runInV8Stack([&](jsg::V8StackScope& stackScope) {
  // ...
});
```
  • Loading branch information
jasnell committed Jan 22, 2024
1 parent 8c3cc45 commit 7a5188f
Show file tree
Hide file tree
Showing 16 changed files with 2,081 additions and 2,005 deletions.
123 changes: 63 additions & 60 deletions src/workerd/api/actor-state-test.c++
Original file line number Diff line number Diff line change
Expand Up @@ -33,51 +33,53 @@ JSG_DECLARE_ISOLATE_TYPE(ActorStateIsolate, ActorStateContext);
KJ_TEST("v8 serialization version tag hasn't changed") {
jsg::test::Evaluator<ActorStateContext, ActorStateIsolate> e(v8System);
ActorStateIsolate &actorStateIsolate = e.getIsolate();
jsg::V8StackScope stackScope;
ActorStateIsolate::Lock isolateLock(actorStateIsolate, stackScope);
isolateLock.withinHandleScope([&] {
auto v8Context = isolateLock.newContext<ActorStateContext>().getHandle(isolateLock);
auto contextScope = isolateLock.enterContextScope(v8Context);

auto buf = serializeV8Value(isolateLock, isolateLock.boolean(true));

// Confirm that a version header is appropriately written and that it contains the expected
// current version. When the version increases, we need to write a v8 patch that allows it to
// continue writing data at the old version so that we can do a rolling upgrade without any bugs
// caused by old processes failing to read data written by new ones.
KJ_EXPECT(buf[0] == 0xFF);
KJ_EXPECT(buf[1] == 0x0F); // v8 serializer version

// And this just confirms that the deserializer agrees on the version.
v8::ValueDeserializer deserializer(isolateLock.v8Isolate, buf.begin(), buf.size());
auto maybeHeader = deserializer.ReadHeader(isolateLock.v8Context());
KJ_EXPECT(jsg::check(maybeHeader));
KJ_EXPECT(deserializer.GetWireFormatVersion() == 15);

// Just for kicks, make sure it deserializes properly too.
KJ_EXPECT(deserializeV8Value(isolateLock, "some-key"_kj, buf).isTrue());
jsg::runInV8Stack([&](jsg::V8StackScope& stackScope) {
ActorStateIsolate::Lock isolateLock(actorStateIsolate, stackScope);
isolateLock.withinHandleScope([&] {
auto v8Context = isolateLock.newContext<ActorStateContext>().getHandle(isolateLock);
auto contextScope = isolateLock.enterContextScope(v8Context);

auto buf = serializeV8Value(isolateLock, isolateLock.boolean(true));

// Confirm that a version header is appropriately written and that it contains the expected
// current version. When the version increases, we need to write a v8 patch that allows it to
// continue writing data at the old version so that we can do a rolling upgrade without any
// bugs caused by old processes failing to read data written by new ones.
KJ_EXPECT(buf[0] == 0xFF);
KJ_EXPECT(buf[1] == 0x0F); // v8 serializer version

// And this just confirms that the deserializer agrees on the version.
v8::ValueDeserializer deserializer(isolateLock.v8Isolate, buf.begin(), buf.size());
auto maybeHeader = deserializer.ReadHeader(isolateLock.v8Context());
KJ_EXPECT(jsg::check(maybeHeader));
KJ_EXPECT(deserializer.GetWireFormatVersion() == 15);

// Just for kicks, make sure it deserializes properly too.
KJ_EXPECT(deserializeV8Value(isolateLock, "some-key"_kj, buf).isTrue());
});
});
}

KJ_TEST("we support deserializing up to v15") {
jsg::test::Evaluator<ActorStateContext, ActorStateIsolate> e(v8System);
ActorStateIsolate &actorStateIsolate = e.getIsolate();
jsg::V8StackScope stackScope;
ActorStateIsolate::Lock isolateLock(actorStateIsolate, stackScope);
isolateLock.withinHandleScope([&] {
auto v8Context = isolateLock.newContext<ActorStateContext>().getHandle(isolateLock);
auto contextScope = isolateLock.enterContextScope(v8Context);

kj::Vector<kj::StringPtr> testCases;
testCases.add("54");
testCases.add("FF0D54");
testCases.add("FF0E54");
testCases.add("FF0F54");

for (const auto& hexStr : testCases) {
auto dataIn = kj::decodeHex(hexStr.asArray());
KJ_EXPECT(deserializeV8Value(isolateLock, "some-key"_kj, dataIn).isTrue());
}
jsg::runInV8Stack([&](jsg::V8StackScope& stackScope) {
ActorStateIsolate::Lock isolateLock(actorStateIsolate, stackScope);
isolateLock.withinHandleScope([&] {
auto v8Context = isolateLock.newContext<ActorStateContext>().getHandle(isolateLock);
auto contextScope = isolateLock.enterContextScope(v8Context);

kj::Vector<kj::StringPtr> testCases;
testCases.add("54");
testCases.add("FF0D54");
testCases.add("FF0E54");
testCases.add("FF0F54");

for (const auto& hexStr : testCases) {
auto dataIn = kj::decodeHex(hexStr.asArray());
KJ_EXPECT(deserializeV8Value(isolateLock, "some-key"_kj, dataIn).isTrue());
}
});
});
}

Expand Down Expand Up @@ -107,27 +109,28 @@ KJ_TEST("wire format version does not change deserialization behavior on real da

jsg::test::Evaluator<ActorStateContext, ActorStateIsolate> e(v8System);
ActorStateIsolate &actorStateIsolate = e.getIsolate();
jsg::V8StackScope stackScope;
ActorStateIsolate::Lock isolateLock(actorStateIsolate, stackScope);
isolateLock.withinHandleScope([&] {
auto v8Context = isolateLock.newContext<ActorStateContext>().getHandle(isolateLock);
auto contextScope = isolateLock.enterContextScope(v8Context);

// Read in data line by line and verify that it round trips (serializes and then deserializes)
// back to the exact same data as the input.
std::string hexStr;
const auto key = "some-key"_kj;
while (std::getline(file, hexStr)) {
auto dataIn = kj::decodeHex(kj::ArrayPtr(hexStr.c_str(), hexStr.size()));
KJ_EXPECT(!dataIn.hadErrors, hexStr);

auto oldVal = oldDeserializeV8Value(isolateLock, dataIn);
auto oldOutput = serializeV8Value(isolateLock, oldVal);

auto newVal = deserializeV8Value(isolateLock, key, dataIn);
auto newOutput = serializeV8Value(isolateLock, newVal);
KJ_EXPECT(oldOutput == newOutput, hexStr);
}
jsg::runInV8Stack([&](jsg::V8StackScope& stackScope) {
ActorStateIsolate::Lock isolateLock(actorStateIsolate, stackScope);
isolateLock.withinHandleScope([&] {
auto v8Context = isolateLock.newContext<ActorStateContext>().getHandle(isolateLock);
auto contextScope = isolateLock.enterContextScope(v8Context);

// Read in data line by line and verify that it round trips (serializes and then deserializes)
// back to the exact same data as the input.
std::string hexStr;
const auto key = "some-key"_kj;
while (std::getline(file, hexStr)) {
auto dataIn = kj::decodeHex(kj::ArrayPtr(hexStr.c_str(), hexStr.size()));
KJ_EXPECT(!dataIn.hadErrors, hexStr);

auto oldVal = oldDeserializeV8Value(isolateLock, dataIn);
auto oldOutput = serializeV8Value(isolateLock, oldVal);

auto newVal = deserializeV8Value(isolateLock, key, dataIn);
auto newOutput = serializeV8Value(isolateLock, newVal);
KJ_EXPECT(oldOutput == newOutput, hexStr);
}
});
});
}

Expand Down
Loading

0 comments on commit 7a5188f

Please sign in to comment.