Skip to content

Concurrent modification of coverage map when merging multiple files #1424

@ssams

Description

@ssams

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions