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

Make releaseProxy asynchronous #625

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

garethj2
Copy link

@garethj2 garethj2 commented Mar 2, 2023

When I was using Deno with comlink, its testing framework was detecting that MessagePorts weren't being closed properly. In an earlier version of comlink, one could await a call to releaseProxy to ensure resources were properly disposed. It seems like this as been lost along the way with the latest changes.

Indeed the finalizers are a great addition but to make the tests clean, I still want to call releaseProxy accordingly when managing resources. In order for this to happen, releaseProxy needs to return a promise so it can be awaited upon.

@google-cla
Copy link

google-cla bot commented Mar 2, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@garethj2
Copy link
Author

It would be great to get this reviewed and submitted if possible.

@defunctzombie
Copy link
Contributor

Isn't the idea of releaseProxy to release references without caring about whether the other side responds or not?

Note: None of the other code uses async/await - I don't know if that's be design (sometimes done for performance) or not but wanted to point that out as well.

@garethj2
Copy link
Author

@defunctzombie true but there are occasions where you want to watch for the proxy to be released.

My main use case was running unit tests in Deno. Without waiting for releaseProxy to complete, deno's unit test framework would complain about resource leaks. For most of the tests, I want to be able to start everything up and then shut everything down cleanly.

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