-
Notifications
You must be signed in to change notification settings - Fork 5
Add kernel service object support #563
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Kernel Service Invocation Errors
The #invokeKernelService method has two issues:
- The
try-catchblock is too narrow, only wrapping the service method invocation. Errors during message deserialization (kunser) or parameter validation are not caught, which can crash the kernel or leave calling vat promises hanging indefinitely. - Service methods are invoked with
thisbound tonullviamethodFunction.apply(null, args). This can break methods that rely on theirthiscontext to access properties or other methods of the service object.
packages/ocap-kernel/src/Kernel.ts#L667-L711
ocap-kernel/packages/ocap-kernel/src/Kernel.ts
Lines 667 to 711 in 9b647a9
| async #invokeKernelService(target: KRef, message: Message): Promise<void> { | |
| const kernelService = this.#kernelServicesByObject.get(target); | |
| if (!kernelService) { | |
| throw Error(`no registered service for ${target}`); | |
| } | |
| const { methargs, result } = message; | |
| const [method, args] = kunser(methargs) as [string, unknown[]]; | |
| assert.typeof(method, 'string'); | |
| if (result) { | |
| assert.typeof(result, 'string'); | |
| } | |
| // eslint-disable-next-line @typescript-eslint/no-unsafe-function-type | |
| const service = kernelService.service as Record<string, Function>; | |
| const methodFunction = service[method]; | |
| if (methodFunction === undefined) { | |
| if (result) { | |
| this.#kernelQueue.resolvePromises('kernel', [ | |
| [result, true, kser(Error(`unknown service method '${method}'`))], | |
| ]); | |
| } else { | |
| this.#logger.error(`unknown service method '${method}'`); | |
| } | |
| return; | |
| } | |
| assert.typeof(methodFunction, 'function'); | |
| assert(Array.isArray(args)); | |
| try { | |
| // eslint-disable-next-line prefer-spread | |
| const resultValue = await methodFunction.apply(null, args); | |
| if (result) { | |
| this.#kernelQueue.resolvePromises('kernel', [ | |
| [result, false, kser(resultValue)], | |
| ]); | |
| } | |
| } catch (problem) { | |
| if (result) { | |
| this.#kernelQueue.resolvePromises('kernel', [ | |
| [result, true, kser(problem)], | |
| ]); | |
| } else { | |
| this.#logger.error('error in kernel service method:', problem); | |
| } | |
| } | |
| } |
Was this report helpful? Give feedback by reacting with 👍 or 👎
grypez
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR resolves the essential issue of making kernel functionality available to user code.
The ad hoc integration with existing components invites a few follow up issues:
- "The Kernel" is represented in various places by either the string
'kernel'or theundefinedvalue, for examplesetPromiseDecideruses'kernel'to represent the kernel, whileKernelQueue.resolvePromisesusesundefined; this is worrying. Probably this is settled by including the'kernel'literal as anEndpointId; this would have consequences elsewhere. - Although the implementations may vary between platforms, the service registrar should at least furnish a dependable interface to the cluster developer; at present the bootstrap method receives as its services object a potentially monstrous duck. This ties into improving vat development more broadly.
These things being noted, the implementations of user code syscalls that this PR enables can likely depend upon the code in this commit without disruption.
I approve.
This PR adds support for kernel-privlleged (i.e., syscall-like) capabilities that can be messaged from vats just like any other imported object reference. While motivated by the need to provide a way for vats to access the recently added or soon to be added revocation and ocap URL manipulation services, it is specifically designed to be extensible, so that new such capabilities can be added in the future without modification to the kernel itself.
A new kernel method,
registerKernelServiceObject(name: string, service: object): voidallows code hosting a kernel to install such services and register them by name. This must be called for a service prior to loading any subclusters that would make use of it. The service itself is created as a regularFarobject (basically, an object whose only own properties are functions, i.e., methods) that can have whatever methods you like. When invoked, parameters are deserialized using thekernel-marshalpackage, so values that are exported object references will be tokens that you can pass as parameters in subsequent messages that the kernel service sends (using the kernel'squeueMessagemethod) or you can interrogate their associated kref with thekrefOffunction from thekernel-marshalpackage (the tests here give an example of the latter). The return value from a service method resolves the promise that the requesting vat holds from the original message send.The cluster configuration object has been augmented with a new optional
servicesproperty, whose value is an array of the names of any registered service capabilities you wish to make available to the subcluster being configured. These services will be passed in a new second parameter to thebootstrapmethod. This parameter will be an object whose property names are the service names and whose property values are the corresponding service objects. The bootstrap vat can use these itself, or pass them on to other vats in the cluster, just as it would with the references to vat root objects that are passed in the first parameter tobootstrap. Vats can send messages to these objects in the normal way.