-
-
Notifications
You must be signed in to change notification settings - Fork 366
Description
When merging multiple coverage files (e.g. from the temp-dir when generating reports or running the merge command) in getCoverageMapFromAllCoverageFiles(...), processing of individual files is parallelized and uses multiple threads:
Lines 421 to 431 in ab7c53b
| const map = libCoverage.createCoverageMap({}) | |
| const files = await this.coverageFiles(baseDirectory) | |
| await pMap( | |
| files, | |
| async f => { | |
| const report = await this.coverageFileLoad(f, baseDirectory) | |
| map.merge(report) | |
| }, | |
| { concurrency: os.cpus().length } | |
| ) |
However, the current implementation causes multiple threads to potentially call map.merge(...) concurrently, leading to undesired side effects unless the merge function is implemented in a thread safe manner.
The used p-map package and in particular it's "sister package" p-map-series also seem to document that this is not a safe usage, as the separate p-map-series package states in its documentation:
Useful as a side-effect mapper. Use p-map if you don't need side-effects, as it's concurrent.
=> p-map on its own is also not intended to be used with side-effects.
I can't provide a working example demonstrating failure of this snippet at the moment as I've just stumbled across this whilst working on something else but fortunately did not run into a specific bug due to this (yet) - as this is about a race condition it would be difficult to reproduce anyway.