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

EventEmitter.on exits function after async iteration of events #32360

Closed
mitsos1os opened this issue Mar 19, 2020 · 5 comments
Closed

EventEmitter.on exits function after async iteration of events #32360

mitsos1os opened this issue Mar 19, 2020 · 5 comments

Comments

@mitsos1os
Copy link
Contributor

  • Version:v.12.16.1
  • Platform: Linux 5.3.0-42-generic fix LICENSE #34~18.04.1-Ubuntu SMP Fri Feb 28 13:42:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
  • Subsystem:

What steps will reproduce the bug?

I am not sure if this is actually an issue or it works as expected, but the newly introduced EventEmiter.on functionality used in a for await, exits the function in which is called after iteration, without running the following commands. Notice the snippet from the actual documentation with an extra log line added:

const { on, EventEmitter } = require('events');

(async () => {
  const ee = new EventEmitter();

  // Emit later on
  process.nextTick(() => {
    ee.emit('foo', 'bar');
    ee.emit('foo', 42);
  });

  for await (const event of on(ee, 'foo')) {
    // The execution of this inner block is synchronous and it
    // processes one event at a time (even with await). Do not use
    // if concurrent execution is required.
    console.log(event); // prints ['bar'] [42]
  }
  console.log('The End'); // This is never printed...!
})();

However this is not the case using a regular AsyncIterator:

(async () => {
  
  async function* asyncGenerator() {
    let i = 0;
    while (i < 3) {
      yield i++;
    }
  }

  for await (const event of asyncGenerator()) {
    // The execution of this inner block is synchronous and it
    // processes one event at a time (even with await). Do not use
    // if concurrent execution is required.
    console.log(event); // prints ['bar'] [42]
  }
  console.log('The End'); // This is printed normally!
})();

So that is why I believe it is a problem with EventEmitter.on functionality...

What is the expected behavior?

I believe it should continue execution of code after the loop

What do you see instead?

Code following loop is not run.

@himself65
Copy link
Member

node/lib/events.js

Lines 697 to 700 in ffdf1de

// Wait until an event happens
return new Promise(function(resolve, reject) {
unconsumedPromises.push({ resolve, reject });
});

return new Promse will make a pause and wait until the process exit

@himself65
Copy link
Member

himself65 commented Mar 19, 2020

same as

;(async () => {
  await new Promise(() => {})
  console.log('The End') // This is never printed...!
})()

@Jack-Works
Copy link

same as

;(async () => {
  await new Promise(() => {})
  console.log('The End') // This is never printed...!
})()

If you read the ee from the closure and emit one new event, the async iterator will advance by one step.

@himself65
Copy link
Member

same as

;(async () => {
  await new Promise(() => {})
  console.log('The End') // This is never printed...!
})()

If you read the ee from the closure and emit one new event, the async iterator will advance by one step.

yes, sure. I just told what caused it.

@mitsos1os
Copy link
Contributor Author

mitsos1os commented Mar 19, 2020

Ok guys, I understand now.
It exits because nothing else is registered in the event loop and not because the iteration of AsyncIterable returned by EventEmitter.onwas over done=true. The iterator is always pending for a new event emission so that a new loop turn in for await is triggered.

My misunderstanding was based on the provided documentation that stated

It removes all listeners when exiting the loop.

I mistook this as if it meant that the loop exits, (so iterator is over), after all events registered up to that time are fired.

Closing the issue as invalid.
Thanks!

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

3 participants