Skip to content

Commit

Permalink
fix(swingset): use better async style, improve comment
Browse files Browse the repository at this point in the history
The use of `await` inside a try/finally block would expose a subtle timing
issue (if `thunk()` throws synchronously, the finalizer would run in the same
turn as the thunk, rather than in a subsequent turn). Our recommended style
avoids conditional `await` for just that reason, so this updates
`runWithoutMeteringAsync` to use a better approach.
  • Loading branch information
warner committed Aug 13, 2021
1 parent beb9f59 commit 64e4f2f
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 12 deletions.
10 changes: 5 additions & 5 deletions packages/SwingSet/src/kernel/dummyMeterControl.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@ export function makeDummyMeterControl() {

async function runWithoutMeteringAsync(thunk) {
meteringDisabled += 1;
try {
return await thunk();
} finally {
meteringDisabled -= 1;
}
return Promise.resolve()
.then(() => thunk())
.finally(() => {
meteringDisabled -= 1;
});
}

// return a version of f that runs outside metering
Expand Down
3 changes: 2 additions & 1 deletion packages/SwingSet/src/kernel/liveSlots.js
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,8 @@ function build(
if (type === 'object') {
// Set.delete() metering seems unaffected by presence/absence, but it
// doesn't matter anyway because deadSet.add only happens when
// finializers run, which happens deterministically
// finializers run, and we wrote xsnap.c to ensure they only run
// deterministically (during gcAndFinalize)
deadSet.delete(slot);
droppedRegistry.register(val, slot, val);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,12 @@ function makeMeterControl() {
const limit = globalThis.currentMeterLimit();
const before = globalThis.resetMeter(0, 0);
meteringDisabled += 1;
try {
return await thunk();
} finally {
globalThis.resetMeter(limit, before);
meteringDisabled -= 1;
}
return Promise.resolve()
.then(() => thunk())
.finally(() => {
globalThis.resetMeter(limit, before);
meteringDisabled -= 1;
});
}

// return a version of f that runs outside metering
Expand Down

0 comments on commit 64e4f2f

Please sign in to comment.