ShutdownWrap vs handle.close

I'm trying to understand some of the net code and ran into _final and _destroy.

_final uses a ShutdownWrap while _destroy does a handle.close()

What is the difference?

Not an expert, but... isn't final and destroy fundamentally different?

Final/shutdown is an ordered operation, that after all data currently queued has transmitted, will do an orderly shutdown of the socket indicating no more outbound data will be sent (but not preventing more data inbound).

destroy just closes the underlying OS handle (an fd on unix). The timing of this wrt. any ongoing data being sent or received is indeterminate, and once the fd is closed it will (of course) cease to exist, so can't be used to send or recv any more data.

What happens on destroy() to all the ongoing send/recv queues is AFAICT not well specified by the streams contracte, and not consistently implemented... thus the stream of issues you are finding related to it. streams are about ordered bi-directional (sometimes) data xfer, and destroy just chops through that.

Given this, that one does an orderly async wrapped operation, and one probably goes much more directly towards uv_close() is what I'd expect, is that what you are seeing?

isn't final and destroy fundamentally different?

Indeed.

socket indicating no more outbound data will be sent (but not preventing more data inbound).

Good. This was not clear to me. Though I'm still a little confused why "shutdown" only affects the outbound side, should I think of it as "shutdown/end outbound"?. Is it possible to shutdown inbound only?

destroy just closes the underlying OS handle (an fd on unix)

Got it.

thus the stream of issues you are finding related to it. streams are about ordered bi-directional (sometimes) data xfer, and destroy just chops through that.

Makes sense.

https://linux.die.net/man/2/shutdown

^-- can do either or both directions. close() is similar to doing both directions.

"normal" (orderly) usage is read() until EOF on incoming, write until no more data to send, then shutdown(WR) on outgoing. When both EOF received, and shutdown(WR) have ocurred, then close(). Thats at the syscall layer, but all the stack layers above emulate that basic pattern (because that is ultimately what occurs).

abnormal termination is just close(), at syscall layer. .destroy() (maybe... its not well defined) at the node stream layer.

should I think of it as "shutdown/end outbound"?

yes: .end() maps to shutdown(WR)

Perfect. I think that answered my question. Thanks a lot.

@sam-github

@sam-github @addaleax just one more follow up here, _destroy is currently called before _final in the 'end' handler, i.e. it does not wait for shutdown to be called or completed, is this a potential problem?

pseudo example

function onReadableStreamEnd () {
  this.end() // -> process.nextTick -> _final -> shutdown
  this.destroy() // -> _destroy -> close
}

i.e. allowHalfOpen might not be gracefully closing the handle, and calling shutdown after close might be problematic?

it's probably fixed through #31806 but I'm just curious whether this could be a problem in earlier node versions?

@ronag

@ronag Yeah, I would say that’s a potential problem … ideally, the shutdown would complete first and call the _final callback, right? Otherwise this might result in unexpected ECONNRESET errors?

ideally, the shutdown would complete first and call the _final callback,

Yes, the !allowHalfOpen case should probably not directly call destroy and instead let _final invoke it. But doing so causes another bug, see https://github.com/nodejs/node/pull/31806/files#r382938926.

@addaleax

@addaleax also, deferring destroy to _final causes this #32487 in win10.

I think its a house of cards down there in C++ streams.... TLS is the worse I've seen, because its this complex protocol engine with a top (facing node js) and a bottom (facing another stream... likely a tcp socket, but could be any bi-directional stream), and when a packet comes IN the bottom, it can actually cause stuff to go back OUT even though there is nothing in or out from the js layer yet. And reading from the js layer can cause TLS to wake up and write things out the bottom... it really stresses the "independent bi-directional streams that xform data going in the same direction, but with weak interactions between the in and out directions" paradigm of sockets. It works, seemingly!, but its pretty fragile.

Right. I’d be a big fan of re-writing the TLS implementation from scratch… 😄

I'd be a big fan of re-writing the TLS implementation...

Oh most definitely. It's been on my todo list for a while now, especially having done through the whole QUIC implementation.

I tired to rework the "implicit handshake" to explicit, but never got it quite working, there was always some set of edge test cases that broke when another was fixed, and I ran out of time and energy.

net: wait for shutdown to complete before closing

Is the handle.close and ShutdownWrap callbacks guaranteed to be asynchronous regardless of platform?

I have some vague memory of this not being the case but wanted to check with someone more knowledgable in this area,

@ronag

@ronag handle.close() is always asynchronous and always succeeds. .shutdown() can succeed synchronously and returns 1 in that case; that’s always the case for HTTP/2. It can also fail synchronously for libuv-backed streams, in that case it returns a negative, non-zero libuv error code.

net: wait for shutdown to complete before closing

Navigation Menu

ShutdownWrap vs handle.close #32486

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

ShutdownWrap vs handle.close #32486

Description

Activity

sam-github commented on Mar 25, 2020

ronag commented on Mar 25, 2020

sam-github commented on Mar 25, 2020

sam-github commented on Mar 25, 2020

ronag commented on Mar 25, 2020

ronag commented on Mar 25, 2020

addaleax commented on Mar 25, 2020

ronag commented on Mar 25, 2020

ronag commented on Mar 25, 2020

sam-github commented on Mar 25, 2020

addaleax commented on Mar 25, 2020

jasnell commented on Mar 25, 2020

sam-github commented on Mar 25, 2020

ronag commented on Mar 26, 2020

addaleax commented on Mar 26, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions