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

Intermediate async iterator does not correctly emit readable event, unpredictably #96

Open
gsvarovsky opened this issue Nov 19, 2022 · 2 comments

Comments

@gsvarovsky
Copy link

I'm getting a behaviour that appears to be caused by a residual task scheduler ordering issue, related to #28.

I have a number of unit tests that involve async iterators returned from node-quadstore. In some cases the test can hang and timeout. The hangs are consistently reproducible but unpredictable. If some tests are skipped, the hang happens in a different place; but the tests are fully independent with respect to everything but globals.

Debugging shows that the hang is caused by an async iterator which is not emitting a readable event despite having new items available.

The screenshot shows a debug session in which the readable event is about to be suppressed because the async iterator already has this._readable === true.

I have two reasons to suspect a task ordering issue:

  1. This code is called in the task which was scheduled in handling the new readable event from the source LevelIterator (you can see that the source is readable and that the destination SimpleTransformIterator is not yet readable).
  2. Calling setTaskScheduler(queueMicrotask) early in my setup fixes the issue.

image

I will continue to investigate. I may try to create a test scenario, but it may be more fruitful for me to offer a hypothesis and a fix instead. Notifying you here in case you have any ideas! 🙂

@jeswr
Copy link
Collaborator

jeswr commented Nov 20, 2022

Hi @gsvarovsky thanks for the report! Just so you are aware; I'm working on a rewrite of this library, and that rewrite is more determinate about how the end, and readable events are emitted.

I cannot guarantee it will be released any time soon as it may take some time to properly review. But in the same token I would also not spend a significant amount of time on test scenarios that are specific to the current design of the code.

@gsvarovsky
Copy link
Author

Thanks @jeswr

Since this will be addressed by your rewrite, but in the meantime can cause hangs in tests and (theoretically) apps, I'm going to release a patch of m-ld that exclusively uses queueMicrotask.

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

No branches or pull requests

2 participants