I think this would lead to the event handler being called with a wrong this value, and I think this might not work if there is more than one listener (because stream._events.close would be an array then).

Could we use the listener in the lines below, and call stream.emit('close') from there?

I see. It might not work when events is an array.

I want to fix.
createSocket create new Socket object and sharing only handle of old socket.
Therefore, we can't call old close events.
How do i fix it best ?

https://github.com/nodejs/node/blob/5a3e445304f3920590b2ae1d1237fe7615572a44/lib/internal/child_process.js#L274
https://github.com/nodejs/node/blob/5a3e445304f3920590b2ae1d1237fe7615572a44/lib/internal/child_process.js#L377

Therefore, we can't call old close events.

I’m not sure I understand this – Can you explain more? i.e. what wouldn’t work if we add stream.emit('close'); in the callback below?

I was misunderstanding. It could use stream.emit('close'), if new stream object have old stream object instead of having _events.

I am going to fix.

if (stream._stdio) {
  stream._stdio.emit('close');
}

  handle: handle,
- _events: events 
+ _stdio: stdio

Shouldn’t this go in the stream.socket.on('close', () => { handler below?

@addaleax I could do it. I fixed and push this code. Please check it.

So … the code above (https://github.com/nodejs/node/pull/22892/files#diff-f45eb699237c2e38dc9b49b588933c11R923) reads like stdio could be:

• A native stream handle
• An object with a handle property which is a native stream handle
• An object with a _handle property which is a native stream handle

The path that is typically taken for e.g. net.Sockets would be the third one, I think. We should probably look into deprecating at least the first two, but … all that’s a different story and not really for this PR.

What matters is that stdio does not need to be any kind of particular object; most of the time, it’s a net.Socket or similar, but it could also just be { handle: someNativeHandle }. Even though that’s a bad idea imo, the code above points to this being a possibility. This means that the stream._stdio.emit() call above might fail, because emit() might not be available?

I guess a simple solution would be to turn the if (stream._stdio) { conditional into if (stream._stdio && typeof stream._stdio.emit === 'function') { ?

I see. I will fix other PR.

I came up with a way when object is pushed. How about this?
Should it be judged when emit()?

const a = {
  type: 'wrap',
  wrapType: getHandleWrapType(handle),
  handle: handle
};

if (typeof stdio.emit === 'function') {
  a._stdio = stdio;
}

acc.push(a);

Or how about if (stream._stdio instanceof EventEmitter)?

@koh110 I’m not sure. instanceof EventEmitter is probably fine, too?

I would prefer for the _stdio property to be present consistently, though – either it’s always there or always missing.

@addaleax Yeah, you right. I completely agree. I was wrong.

I would prefer for the _stdio property to be present consistently,

I feel that if (stream._stdio && stream._stdio instanceof EventEmitter) { is more meaningful than if(stream._stdio && typeof stream._stdio.emit === 'function') {. EventEmitter may be changed to WritableStream. What do you think about it?