`pidusage` doesn't resolve if `pids` array gets modified meanwhile `updateCpu()` gets called

[piranna]

API Platform version(s) affected:

Probably all, detected while using Ubuntu 23.10

Description

pids array argument is checked not being zero length at https://github.com/soyuka/pidusage/blob/61480f745a3f4dd780118b6aabf00bc1f58d5d87/lib/stats.js#L71-L73, but it's passed and used as reference in all the code, so when calling async function updateCpu() at https://github.com/soyuka/pidusage/blob/61480f745a3f4dd780118b6aabf00bc1f58d5d87/lib/procfile.js#L125, it can be possible that the array gets modified in application code (for example, due to a race condition), so when calling its callback, pids can be a zero length array without getting noticed. This has the effect that fns object will be empty instead of being filled at https://github.com/soyuka/pidusage/blob/61480f745a3f4dd780118b6aabf00bc1f58d5d87/lib/procfile.js#L131-L135, and later when calling to parallel() helper, it will never call to the each() function instead of doing it at https://github.com/soyuka/pidusage/blob/61480f745a3f4dd780118b6aabf00bc1f58d5d87/lib/helpers/parallel.js#L30-L34, and the done() callback will never be called back.

Possible Solution

There are several, not incompatible alternatives:

• create a new array instead of modify the original one at https://github.com/soyuka/pidusage/blob/61480f745a3f4dd780118b6aabf00bc1f58d5d87/lib/stats.js#L75-L80 when parsing the pid strings. Using Array.map() here is just enough. Problem is, if process exits later, maybe we would be trying to access to some info of a non-existing process, that could lead to some other bigger problems down the line, for example when reading procfile, but maybe that would be done on purpose
• check (again) the array is still non zero length at updateCpu() callback, and call done() with an error if that happens. Alternatively, return an empty array instead
• check that fns is not an empty array or object in [parallel()helper](https://github.com/soyuka/pidusage/blob/61480f745a3f4dd780118b6aabf00bc1f58d5d87/lib/helpers/parallel.js#L12), and calldone()` with an error if that happens. That would be the most versatile solution, but would detect it later. Alternatively, return an empty array instead

Personally I would implement at least 2 and 3 for safety, and if we want to get procfile errors on the benefict of having an inmutable input data, also number 1, maybe returning a structure similar to the one returned by Promise.allSettled() that combines both results and errors at the same time.

[soyuka]

would love a pr if you're up to it!

[piranna]

Just to fic this, or a full redactor? There are some needed updates, but others like using Promise.allSettled() would be backward incompatible with older versions of Node.js... but for maintained ones It would be totally safe.

[piranna]

In fact, i'm thinking the best option would be to return an empty object or array if it's zero length, also when provided like this, so i would remové the throwed exceptuon