throat icon indicating copy to clipboard operation
throat copied to clipboard

bug: Throat causes a halt in execution under certain conditions

Open bjornua opened this issue 2 years ago • 2 comments

Describe the bug

Throat causes a halt in execution under certain conditions

To Reproduce

const _ = require('lodash');
const throat = require('throat');

async function main() {
  const worker = throat(4, async function (page) {});
  const pagesChunks = _.range(10).map((n) => _.range(100));

  let awaitCount = 0;
  for (const pagesChunk of pagesChunks) {
    console.log(`Await ${awaitCount++}`);
    await Promise.all(pagesChunk.map(worker));
  }

  console.log('Done');
}

main();

This outputs:

Await 0
Await 1
Await 2

Expected behaviour

Expected output:

Await 0
Await 1
Await 2
Await 3
Await 4
Await 5
Await 6
Await 7
Await 8
Await 9
Done

Versions

  • node version: v14.17.5
  • npm/yarn version: 6.14.14
  • package version: throat-6.0.1

Additional context

Messing around with the length of pagesChunk or size can make the script complete normally, or halt earlier or later in the iterations.

bjornua avatar Sep 06 '21 21:09 bjornua

We faced the same issue with this library. @ForbesLindesay please take a look at this - the library has a critical bug and it has 18m+ weekly downloads

andriimaryskevych avatar Apr 01 '22 10:04 andriimaryskevych

Can confirm and isolate this a bit more also without the chunking above.

And yes the blocksize is the magic number 64 https://github.com/ForbesLindesay/throat/blob/master/index.js#L105

async function demoExitBug() {
  const limitedConcurrency = throat(1);
  for (let i = 0; i < 100; i += 1) {
    console.log('working on', i);
    await Promise.all([
      limitedConcurrency(async () => true), // with just one it would complete
      limitedConcurrency(async () => true), // concurrency + 1 = only 64 ok
      // limitedConcurrency(async () => true), // concurrency + 2 = only 32 ok
    ]);
  }
  console.log('Done!'); // This will not be reached in 6.0.1
}
demoExitBug();

Looks like this was introduced in 6.0.1 https://github.com/ForbesLindesay/throat/commit/8fd7a02dce52ce26743b368f62a33f8752c68a4e#diff-e727e4bdf3657fd1d798edcd6b099d6e092f8573cba266154583a746bba0f346R123

peterlundberg avatar May 12 '22 18:05 peterlundberg

Sorry, didn't notice the issue. Pinging me on twitter is often good for super high priority issues like this. I'll push out a fix shortly.

ForbesLindesay avatar Jan 03 '23 12:01 ForbesLindesay

This is resolved in v6.0.2

ForbesLindesay avatar Jan 03 '23 13:01 ForbesLindesay