这是indexloc提供的服务,不要输入任何密码
Skip to content

Do not patch the global child_process module #73

@demurgos

Description

@demurgos

Hi,
I'm currently looking at providing a Node API for c8, a code coverage tool using V8's Profiler.
The current code uses spawn-wrap. Since spawn-wrap modifies the global child_process module, it is not safe to use it in a library: concurrent code trying to spawn processes will have unexpected results. This prevents running multiple coverages in parallel, or coverage and some other consumer tasks, inside the same Node process.

The current workaround would be to run each instance of c8 in its own process (independent child_process). This is very heavy-handed: it introduces friction and hurts composability. It means that the API must communicate with a subprocess so only serializable messages can be sent.

I propose to do something similar to graceful-fs@4. The version 3 modified the global fs module while version 4 only returned a wrapper.

Here is what the updated API would look like:

// Default to safe version (breaking change)
module.exports = safeWrap;
// Alias so the API can be a simple namespace
module.exports.wrap = safeWrap;
// Keep the old behavior available to ease transition, eventually mark as deprecated?
module.exports.wrapGlobal = wrap;
// Helper function to know if the global child_process is currently wrapped
module.exports.isGlobalWrapped = ...;

safeWrap(options) {
  const spawn = ...;
  const spawnSync = ...;
  // Start with a subset of the `child_process` API,
  // eventually extend it to return a full `child_process`-like object.
  return {spawn, spawnSync};
}

// We could also imagine a function like "wrapify` similar to "gracefulify"
// You can pass it a `child_process`-like object and it would mutate it
// by adding the wrappers in-place.

As stated in the example, this would introduce a breaking change but I believe that it would bring better behavior. The old behavior would still be accessible so transition would be easy.
The breaking change would also be the occasion to require Node 4+: there are other comments/issues mentioning problems with older Node versions. It would also simplify the current feature-detection mechanisms. This is not my main point though, I'm just proposing it by the way
What is needed is a way to spawn concurrent process-trees that do not interfere with each other, without isolating each root in its own sub-process.


Edit: /cc @bcoe @isaacs @sindresorhus

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