Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(daemon): Expose cancellation context in caplets #2079

Merged
merged 3 commits into from
Feb 22, 2024

Conversation

rekmarks
Copy link
Contributor

@rekmarks rekmarks commented Feb 15, 2024

Updates #2086

Exposes the context object to confined and unconfined caplets by passing it as a second argument to their make() function.

@rekmarks rekmarks force-pushed the rekmarks-context-in-caplet branch from 7ad5bed to 58dbd3a Compare February 15, 2024 08:07
@rekmarks rekmarks changed the base branch from endo to master February 15, 2024 23:12
@rekmarks rekmarks force-pushed the rekmarks-context-in-caplet branch 5 times, most recently from fd7c24a to 400a09e Compare February 16, 2024 04:38
@rekmarks rekmarks force-pushed the rekmarks-context-in-caplet branch 2 times, most recently from 2202b5f to e9c42be Compare February 21, 2024 06:02
@rekmarks rekmarks changed the base branch from master to rekmarks-mutually-exclusive-formula-graph-mutations February 21, 2024 06:02
@rekmarks rekmarks force-pushed the rekmarks-context-in-caplet branch 3 times, most recently from 2a1889c to 8b10d5f Compare February 22, 2024 01:29
@rekmarks rekmarks force-pushed the rekmarks-mutually-exclusive-formula-graph-mutations branch 2 times, most recently from afdfa21 to c4f5572 Compare February 22, 2024 01:38
@rekmarks rekmarks force-pushed the rekmarks-context-in-caplet branch from 8b10d5f to 815f677 Compare February 22, 2024 01:39
@rekmarks rekmarks force-pushed the rekmarks-mutually-exclusive-formula-graph-mutations branch from c4f5572 to b2522bf Compare February 22, 2024 01:45
@rekmarks rekmarks force-pushed the rekmarks-context-in-caplet branch 3 times, most recently from 355e206 to dc6f91b Compare February 22, 2024 05:24
@rekmarks rekmarks force-pushed the rekmarks-mutually-exclusive-formula-graph-mutations branch from d9c865c to fcd9093 Compare February 22, 2024 16:18
@rekmarks rekmarks force-pushed the rekmarks-context-in-caplet branch from dc6f91b to 0affff1 Compare February 22, 2024 16:23
Base automatically changed from rekmarks-mutually-exclusive-formula-graph-mutations to master February 22, 2024 18:36
kriskowal and others added 2 commits February 22, 2024 10:37
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.
@rekmarks rekmarks force-pushed the rekmarks-context-in-caplet branch from 0affff1 to 7be3356 Compare February 22, 2024 18:38
@rekmarks rekmarks marked this pull request as ready for review February 22, 2024 18:38
@rekmarks rekmarks requested a review from kriskowal February 22, 2024 18:41
Copy link
Member

@kriskowal kriskowal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please consider the questions. I do not need to review again.

packages/daemon/src/mutex.js Outdated Show resolved Hide resolved
packages/daemon/src/mutex.js Outdated Show resolved Hide resolved
@rekmarks rekmarks force-pushed the rekmarks-context-in-caplet branch from 9a81d03 to d346078 Compare February 22, 2024 19:24
Synchronizes the host's `cancel()` with the formula graph by awaiting
the formula graph mutex in a new daemon method, `cancelValue()`. Modifies
the mutex implementation to permit calling `enqueue()` without
specifying function to call.
@rekmarks rekmarks force-pushed the rekmarks-context-in-caplet branch from d346078 to 8a52453 Compare February 22, 2024 19:36
@rekmarks rekmarks merged commit da516dd into master Feb 22, 2024
14 checks passed
@rekmarks rekmarks deleted the rekmarks-context-in-caplet branch February 22, 2024 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants