nodejs / nodejs/node

regression: child_process stdin pipe close event not emitted

Đang mở
#25,131 11 bình luận 0 reaction 0 người được giao Xem trên GitHub

Chưa có ai nhận issue này.

child_process confirmed-bug net stream
Ngôn ngữ chính
JavaScript
Star
122k
Fork
37.4k
Merge trung bình
4 ngày 3 giờ
Pull request đã merge (30 ngày)
272

Mô tả

Summary
  • Version: v8.12.0 and newer, all v10.x and all v11.x
  • Platform: Linux and macOS amd64
  • Subsystem: child_process

Since #18701 Node.js doesn't emit a close event when a child process closes its stdin pipe. The close is only detected if the parent tries to write to the child's stdin and receives an EPIPE.

The original behaviour is preferable because it allows immediate detection of the close event, rather than waiting for a failed write.

Reproducer

This works as expected in v8.11.4 and fails in all newer versions of v8.x, as well as all v10.x and all v11.x.

const {spawn} = require('child_process');

const cp = spawn(
    'node', [
        '-e',
        'fs.closeSync(0); setTimeout(() => {}, 2000)'
    ],
    {stdio: ['pipe', 'inherit', 'inherit']}
)

setTimeout(() => {
    console.log('BUGGY! stdin close event was not emitted')
    process.exit(1);
}, 1000);

cp.stdin.on('close', () => {
    console.log('Ok!')
    process.exit(0);
});
Problem detail

As far as I can tell the only way the close event can be emitted is if the Socket constructor calls this.read(0). This triggers onStreamRead() to be called, which does stream.push(null) and eventually results in the close event. This is a little weird because stdin isn't a readable stream :)

In Node 8.11.4 this worked because child_process.js:createSocket() called the Socket constructor with options.readable === undefined. So the Socket constructor sees that options.readable !== false and runs this.read(0)

In PR #18701 createSocket() was changed to call the Socket constructor with options.readable === true. This stops this.read(0) from being called and the close event is not emitted.

Hacky solution
--- a/lib/internal/child_process.js
+++ b/lib/internal/child_process.js
@@ -286,3 +286,3 @@ function flushStdio(subprocess) {
 function createSocket(pipe, readable) {
-  return net.Socket({ handle: pipe, readable, writable: !readable });
+  return net.Socket({ handle: pipe, readable, writable: !readable, childProcess: true });
 }
--- a/lib/net.js
+++ b/lib/net.js
@@ -315,3 +315,5 @@ function Socket(options) {
   // buffer.  if not, then this will happen when we connect
-  if (this._handle && options.readable !== false) {
+  if (options.childProcess) {
+    this.read(0);
+  } else if (this._handle && options.readable !== false) {
     if (options.pauseOnCreate) {

This passes the full test suite with a minor change to test-pipewrap.js. I haven't raised a PR because I'm sure somebody could think of a better solution. If someone can point me in the right direction I'm happy to try and implement it.

Thanks :)

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Hướng nghiên cứu

Bắt đầu với lib/internal/child_process.js và lib/net.js, sau đó so sánh thay đổi trong việc tạo socket từ #18701. Chạy reproducer child_process được cung cấp và kiểm tra test-pipewrap.js, bao gồm thay đổi nhỏ được đề cập trong issue. Được xem là hoàn tất khi stdin phát ra close nhanh chóng lúc tiến trình con đóng pipe và các test liên quan đều đạt.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Đánh giá

Công nghệ
javascript, node.js
Lĩnh vực
backend, operating-systems
Loại issue
Lỗi
Độ khó
4/5
Thời gian dự kiến
3-5 ngày
Mức độ hoạt động
Ít trao đổi
Độ rõ ràng
Đặc tả rõ ràng
Mức phù hợp với người mới
48/100

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.