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

Concurrent modification of coverage map when merging multiple files #1424

Open
ssams opened this issue Aug 12, 2021 · 0 comments · May be fixed by #1425
Open

Concurrent modification of coverage map when merging multiple files #1424

ssams opened this issue Aug 12, 2021 · 0 comments · May be fixed by #1425

Comments

@ssams
Copy link

ssams commented Aug 12, 2021

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:

nyc/index.js

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.

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

Successfully merging a pull request may close this issue.

1 participant