-
Notifications
You must be signed in to change notification settings - Fork 9
fix(generators): reject pending promises to prevent runtime deadlock #115
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
… deadlock When using Observable runtime v6.0.0+, generators with undefined initial values would deadlock when their upstream dependencies changed. The runtime waits for the old generator to settle before starting the new one, but the pending promise would never resolve. The fix converts observe() from an async function* to a manual async iterator, allowing return() to immediately reject pending promises without waiting for them to settle first. Adds unit test for promise rejection and integration test reproducing the deadlock scenario with Generators.input and the Observable runtime.
mbostock
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.
Nice, thank you! 👍 This looks about right to me! I’ll probably make some style changes. I agree we need to opt-out of the async generator function so that we can reject the pending promise when the generator is return’ed or throw’ed.
mbostock
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.
I fixed up Generators.queue, too.
When using Observable runtime v6.0.0+, generators with undefined initial values would deadlock when their upstream dependencies changed. The runtime waits for the old generator to settle before starting the new one, but the pending promise would never resolve.
The fix converts observe() from an async function* to a manual async iterator, allowing return() to immediately reject pending promises without waiting for them to settle first.
port of observablehq/stdlib#399, addresses issue raised here observablehq/runtime#376