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

Unhandled rejection can expose GC to user code #8191

Open
mhofman opened this issue Aug 15, 2023 · 3 comments
Open

Unhandled rejection can expose GC to user code #8191

mhofman opened this issue Aug 15, 2023 · 3 comments
Assignees
Labels
bug Something isn't working liveslots requires vat-upgrade to deploy changes SwingSet package: SwingSet

Comments

@mhofman
Copy link
Member

mhofman commented Aug 15, 2023

Describe the bug

The unhandled rejection logic in xsnap-worker is to invoke console.error in the start compartment (supervisor), when the unhandled promise is GCed. Because of the workaround for #6784, and the fact XS internally keeps these unhandled promises in a WeakRef, these are only collected during BOYD.

So far, so good, all is deterministic, but unfortunately console.error uses object-inpect.js to issue a command to log the unhandled rejection, which may invoke user code if the reason value of the rejection can trap (e.g. using accessors or a proxy), which means the user code is now capable of sensing gc. It also means user code is potentially executing during the BOYD delivery.

Expected behavior

User code is not able to sense gc through unhandled rejections

@mhofman mhofman added bug Something isn't working SwingSet package: SwingSet liveslots requires vat-upgrade to deploy changes labels Aug 15, 2023
@mhofman
Copy link
Member Author

mhofman commented Aug 15, 2023

One solution may be to harden object-inspect.js to not trigger any user code. That would require jumping through hoops to only traverse non-proxy objects and only data properties.

@gibson042
Copy link
Member

gibson042 commented Aug 15, 2023

I think that would require many changes to object-inspect.js and/or replacing the native Proxy in workers, but it does feel like the right approach.

@mhofman
Copy link
Member Author

mhofman commented Aug 15, 2023

Yes, we are already planning on replacing the Proxy constructor available to vat code with a stamping one, and gain an IsProxy predicate (the original motivations is passStyleOf checks). See #3905

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working liveslots requires vat-upgrade to deploy changes SwingSet package: SwingSet
Projects
None yet
Development

No branches or pull requests

2 participants