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

Incorporate discussion of turns into explainer #95

Merged
merged 3 commits into from May 9, 2019

Conversation

Projects
None yet
5 participants
@wingo
Copy link
Contributor

commented May 9, 2019

Fixes #78.

wingo added some commits May 9, 2019

@littledan
Copy link
Member

left a comment

This additional explanation is good. However, on the web, finalizers run in a separate queued task, not microtask. They do not interrupt the run of Promise reactions, and your code sample is incorrect. See #86 for context.

@wingo

This comment has been minimized.

Copy link
Contributor Author

commented May 9, 2019

@littledan Thanks for the review, PTAL.

@@ -80,7 +80,7 @@ However this function may return true or false:
async function test(x) {
const ref = new WeakRef(x)
const y = ref.deref();
await true;
await new Promise(resolve => setTimeout(resolve));

This comment has been minimized.

Copy link
@ljharb

ljharb May 9, 2019

Member

await Promise.resolve() should also force a tick

This comment has been minimized.

Copy link
@littledan

littledan May 9, 2019

Member

There are multiple notions of "tick". WeakRefs do not become null after await Promise.resolve(). This was discussed as an open question in the March 2019 TC39 meeting, and discussed it further in this repository. I put some of my rationale in #61 (comment) ; @tschneidereit has also expressed concern about potential compatibility issues between implementations if we used microtask timing.

This comment has been minimized.

Copy link
@ljharb

ljharb May 9, 2019

Member

ah, ok - since setTimeout isn’t in the language, how else would you do this?

This comment has been minimized.

Copy link
@littledan

littledan May 9, 2019

Member

There are embedder hooks in the specification, as there are in several places in the JavaScript specification. Please file an issue if you want to discuss your concerns in more detail.

This comment has been minimized.

Copy link
@ljharb

ljharb May 9, 2019

Member

Embedded hooks don’t mandate the engine provide a mechanism to JS.

This comment has been minimized.

Copy link
@mhofman

mhofman May 9, 2019

Contributor

I've searched and read all issues and comments, but I can't find anywhere an answer to this question I've had before: why is it not allowed to let the host run the ClearKeptObjects and CleanupFinalizationGroup jobs after the current synchronous job?
In that case await Promise.resolve() may be sufficient, but it also may not.
The comments in various issues seem to indicate the host MUST wait a full task turn. I just don't understand why the requirement.
Of course the host should have the liberty to schedule to jobs later if it so chooses.

@littledan littledan merged commit 9d55bdf into tc39:master May 9, 2019

2 checks passed

Travis CI - Pull Request Build Passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@wingo wingo deleted the wingo:mention-pitfalls branch May 9, 2019

}
```

If `x` has a finalizer that invalidates the value returned by `getPtr()`, an implementation is free to collect `x` at any point after the `getPtr()` call, and to finalize it during the `await`, making the `handle(p)` call operate on an invalid pointer. From a language perspective, the fact that `x` was an argument to a function does not prevent it from being collected. Additionally the `return handle(p)` call is a tail call, which an engine may implement as throwing away any stack frame with the `x` binding, which is another opportunity for `x` to become collectible.

This comment has been minimized.

Copy link
@mhofman

mhofman May 9, 2019

Contributor

invalidates the value returned by

Maybe say invalidates the object?
The Javascript reference p itself will stay valid of course, it's the object that is referenced that may be internally invalidated by library logic. So a primitive value can't be invalidated

This comment has been minimized.

Copy link
@mhofman

mhofman May 9, 2019

Contributor

Maybe somethings like this:

In this example, an implementation is free to collect x at any point after the getPtr() call. From a language perspective, the fact that x was an argument to a function does not prevent it from being collected.
If x has a finalizer that invalidates the object returned by getPtr(), it may be called during the await, making the handle(p) call operate on an invalid object.

This comment has been minimized.

Copy link
@erights

erights May 9, 2019

Contributor

What is getPtr?

This comment has been minimized.

Copy link
@mhofman

mhofman May 9, 2019

Contributor

I'm assuming a method that return a wrapper "pointer" object to x, that can be invalidated?

This comment has been minimized.

Copy link
@wingo

wingo May 9, 2019

Author Contributor

Sorry, I think the confusion is from the name getPtr(). I meant to return a member field of x which, when x's finalizer runs, becomes no longer valid; e.g. perhaps x's finalizer frees webassembly memory pointed to by p. I think the name is confusing in that it suggests that you retrieve a pointer to x, but that's not what i meant.

An example of this pattern is the $$ object used in emscripten's embind.

const ref = new WeakRef(x)
const y = ref.deref();
await new Promise(resolve => setTimeout(resolve));
const z = ref.deref();

This comment has been minimized.

Copy link
@erights

erights May 9, 2019

Contributor

This example is wrong. y is a strong pointer to the object. While y is live, the object it cannot be collected. Therefore, derefing a weakref to it, even in a later turn, must give back the object, so long as y is live. Since y and z both participate in the comparison, clearly y is still live at that point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.