Skip to content

Commit

Permalink
test(daemon): Add cancellation context tests
Browse files Browse the repository at this point in the history
Adds unit tests for using the cancellation context in both confined
and unconfined plugins / caplets. Deletes some unused args and fixes
some type issues discovered in the course of work. Also, adds docstrings
to the Context interface in types.d.ts.

In the course of implementation, a bug was discovered. The added tests
only pass due to the addition of a delay that should not be necessary.
The following applies the same to both the confined and unconfined
cases.

The tests are structured as follows:

```
1 const result = E(host).evaluate(
2   'worker',
3   'E(caplet).awaitCancellation()',
4   ['caplet'],
5   ['context-consumer'],
6 );
7 await E(host).cancel('context-consumer');
8 t.is(await result, 'cancelled');
```

The test endo.log reveals the cause:

```
Making make-unconfined:1f440...
Making least-authority:abc51...
Cancelled:
* make-unconfined:1f440...
Making eval:c9ae8...
Making make-unconfined:1f440...
```

Despite receiving the `evaluate` request first and the cancellation
request second, the host processes these requests in reverse order. The
result is that the test hangs. As a temporary fix, a timeout is added on
line 6. This bug will be fixed in a future commit.
  • Loading branch information
rekmarks committed Feb 22, 2024
1 parent 501f96a commit 93ba613
Show file tree
Hide file tree
Showing 5 changed files with 141 additions and 10 deletions.
2 changes: 1 addition & 1 deletion packages/daemon/src/daemon-node-powers.js
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ export const makeSocketPowers = ({ net }) => {
);

/** @type {import('./types.js').SocketPowers['connectPort']} */
const connectPort = ({ port, host, cancelled }) =>
const connectPort = ({ port, host }) =>
new Promise((resolve, reject) => {
const conn = net.connect(port, host, err => {
if (err) {
Expand Down
4 changes: 4 additions & 0 deletions packages/daemon/src/daemon.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ const makeInspector = (type, number, record) =>
list: () => Object.keys(record),
});

/**
* @param {import('./types.js').Context} context - The context to make far.
* @returns {import('./types.js').FarContext} The far context.
*/
const makeFarContext = context =>
Far('Context', {
cancel: context.cancel,
Expand Down
43 changes: 41 additions & 2 deletions packages/daemon/src/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,19 +217,58 @@ export interface Topic<
subscribe(): Stream<TRead, TWrite, TReadReturn, TWriteReturn>;
}

/**
* The cancellation context of a live value associated with a formula.
*/
export interface Context {
cancel: (reason?: unknown, logPrefix?: string) => Promise<void>;
/**
* Cancel the value, preparing it for garbage collection. Cancellation
* propagates to all values that depend on this value.
*
* @param reason - The reason for the cancellation.
* @param logPrefix - The prefix to use within the log.
* @returns A promise that is resolved when the value is cancelled and
* can be garbage collected.
*/
cancel: (reason?: Error, logPrefix?: string) => Promise<void>;

/**
* A promise that is rejected when the context is cancelled.
* Once rejected, the cancelled value may initiate any teardown procedures.
*/
cancelled: Promise<never>;

/**
* A promise that is resolved when the context is disposed. This occurs
* after the `cancelled` promise is rejected, and after all disposal hooks
* have been run.
* Once resolved, the value may be garbage collected at any time.
*/
disposed: Promise<void>;

/**
* @param formulaIdentifier - The formula identifier of the value whose
* cancellation should cause this value to be cancelled.
*/
thisDiesIfThatDies: (formulaIdentifier: string) => void;

/**
* @param formulaIdentifier - The formula identifier of the value that should
* be cancelled if this value is cancelled.
*/
thatDiesIfThisDies: (formulaIdentifier: string) => void;

/**
* @param hook - A hook to run when the value is cancelled.
*/
onCancel: (hook: () => void | Promise<void>) => void;
}

export interface FarContext {
cancel: (reason: Error) => Promise<void>;
whenCancelled: () => Promise<never>;
whenDisposed: () => Promise<void>;
addDisposalHook: Context['onCancel'];
}

export interface InternalExternal<External = unknown, Internal = unknown> {
Expand Down Expand Up @@ -322,7 +361,7 @@ export interface Mail {
listSpecial(): Array<string>;
listAll(): Array<string>;
reverseLookupFormulaIdentifier(formulaIdentifier: string): Array<string>;
cancel(petName: string, reason: unknown): Promise<void>;
cancel(petName: string, reason: Error): Promise<void>;
// Mail operations:
listMessages(): Promise<Array<Message>>;
followMessages(): Promise<FarRef<Reader<Message>>>;
Expand Down
14 changes: 14 additions & 0 deletions packages/daemon/test/context-consumer.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { E, Far } from '@endo/far';

export const make = async (_powers, context) => {
return Far('Context consumer', {
async awaitCancellation() {
try {
await E(context).whenCancelled();
} catch {
return 'cancelled';
}
throw new Error('should have been cancelled');
},
});
};
88 changes: 81 additions & 7 deletions packages/daemon/test/test-endo.js
Original file line number Diff line number Diff line change
Expand Up @@ -566,11 +566,11 @@ test('guest facet receives a message for host', async t => {
await stop(locator);
});

test('direct termination', async t => {
test('direct cancellation', async t => {
const { promise: cancelled, reject: cancel } = makePromiseKit();
t.teardown(() => cancel(Error('teardown')));

const locator = makeLocator('tmp', 'termination-direct');
const locator = makeLocator('tmp', 'cancellation-direct');

await start(locator);
t.teardown(() => stop(locator));
Expand Down Expand Up @@ -643,15 +643,13 @@ test('direct termination', async t => {
['counter'],
),
);

t.pass();
});

test('indirect termination', async t => {
test('indirect cancellation', async t => {
const { promise: cancelled, reject: cancel } = makePromiseKit();
t.teardown(() => cancel(Error('teardown')));

const locator = makeLocator('tmp', 'termination-indirect');
const locator = makeLocator('tmp', 'cancellation-indirect');

await start(locator);
t.teardown(() => stop(locator));
Expand Down Expand Up @@ -731,7 +729,7 @@ test('cancel because of requested capability', async t => {
const { promise: cancelled, reject: cancel } = makePromiseKit();
t.teardown(() => cancel(Error('teardown')));

const locator = makeLocator('tmp', 'termination-via-request');
const locator = makeLocator('tmp', 'cancellation-via-request');

await start(locator);
t.teardown(() => stop(locator));
Expand Down Expand Up @@ -815,6 +813,82 @@ test('cancel because of requested capability', async t => {
);
});

test('unconfined service can respond to cancellation', async t => {
const { promise: cancelled, reject: cancel } = makePromiseKit();
t.teardown(() => cancel(Error('teardown')));

const locator = makeLocator('tmp', 'cancellation-unconfined-response');

await start(locator);
t.teardown(() => stop(locator));

const { getBootstrap } = await makeEndoClient(
'client',
locator.sockPath,
cancelled,
);
const bootstrap = getBootstrap();
const host = E(bootstrap).host();
await E(host).provideWorker('worker');

const capletPath = path.join(dirname, 'test', 'context-consumer.js');
const capletLocation = url.pathToFileURL(capletPath).href;
await E(host).makeUnconfined(
'worker',
capletLocation,
'NONE',
'context-consumer',
);

const result = E(host).evaluate(
'worker',
'E(caplet).awaitCancellation()',
['caplet'],
['context-consumer'],
);
// TODO:cancel This should not be necessary.
// eslint-disable-next-line no-undef
await new Promise(resolve => setTimeout(resolve, 100));
await E(host).cancel('context-consumer');
t.is(await result, 'cancelled');
});

test('confined service can respond to cancellation', async t => {
const { promise: cancelled, reject: cancel } = makePromiseKit();
t.teardown(() => cancel(Error('teardown')));

const locator = makeLocator('tmp', 'cancellation-confined-response');

await start(locator);
t.teardown(() => stop(locator));

const { getBootstrap } = await makeEndoClient(
'client',
locator.sockPath,
cancelled,
);
const bootstrap = getBootstrap();
const host = E(bootstrap).host();
await E(host).provideWorker('worker');

const capletPath = path.join(dirname, 'test', 'context-consumer.js');
await doMakeBundle(host, capletPath, bundleName =>
E(host).makeBundle('worker', bundleName, 'NONE', 'context-consumer'),
);

const result = E(host).evaluate(
'worker',
'E(caplet).awaitCancellation()',
['caplet'],
['context-consumer'],
);
// TODO:cancel This should not be necessary.
// eslint-disable-next-line no-undef
await new Promise(resolve => setTimeout(resolve, 100));
await E(host).cancel('context-consumer');
t.is(await result, 'cancelled');
});

test('make a host', async t => {
const { promise: cancelled, reject: cancel } = makePromiseKit();
t.teardown(() => cancel(Error('teardown')));
Expand Down

0 comments on commit 93ba613

Please sign in to comment.