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

Handler invoked synchronously. Does not protect against reentrancy. #134

Closed
erights opened this issue Jul 13, 2019 · 4 comments · Fixed by Agoric/eventual-send#20
Closed
Labels
eventual-send package: eventual-send

Comments

@erights
Copy link
Member

erights commented Jul 13, 2019

https://github.com/Agoric/eventual-send/blob/010efde2ee97f9aa59be43daebb5eadcace14f2b/src/index.js#L32

This is a bug. At https://github.com/tvcutsem/es-lab/blob/f1417a67741bb727b40f47e6c2b061e20a5d7098/src/ses/nanoq.js#L33 I see that the bug was in my original too. We need to fix this, even though it will disrupt the ordering again.

Thanks to @devsnek at tc39/proposal-wavy-dot#5 (comment) for noticing this.

@erights
Copy link
Member Author

erights commented Jul 13, 2019

@michaelfig
Copy link
Member

I had this defence already before last night but undid it to get SwingSet tests to pass. I will gladly undo my change and get back the half-async behaviour (so that fulfilledHandler is in a later turn), and then make it fully-async (so that unfulfilledHandler is in a later turn too).

@warner
Copy link
Member

warner commented Jul 31, 2019

in case people run into this problem in the future: this bug was fixed in the code that went into the 0.1.10 release, but a problem during the release process meant that the tarball uploaded to NPM included stale dist bundles, which did not include the fix. So the first release that fixes this problem is 0.2.0.

@warner
Copy link
Member

warner commented Dec 4, 2019

in the old repo. this was eventual-send issue 19

@warner warner transferred this issue from Agoric/eventual-send Dec 4, 2019
@warner warner added the eventual-send package: eventual-send label Dec 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
eventual-send package: eventual-send
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants