tls: simplify write mechanism by addaleax · Pull Request #17883 · nodejs/node

The TLS implementation previously had two separate queues for
WriteWrap instances, one for currently active and one for
finishing writes (i.e. where the encrypted output is being written
to the underlying socket).

However, the streams implementation in Node doesn’t allow for
more than one write to be active at a time; effectively,
the only possible states were that:

- No write was active.
- The first write queue had one item, the second one was empty.
- Only the second write queue had one item, the first one was empty.

To reduce overhead and implementation complexity, remove these
queues, and instead store a single `WriteWrap` pointer and
keep track of whether its write callback should be called
on the next invocation of `InvokeQueued()` or not
(which is what being in the second queue previously effected).

PR-URL: #17883
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>

The TLS implementation previously kept a separate buffer for
incoming pieces of data, into which buffers were copied
before they were up for writing.

This removes this buffer, and replaces it with a simple list
of `uv_buf_t`s:

- The previous implementation copied all incoming data into
  that buffer, both allocating new storage and wasting time
  with copy operations. Node’s streams/net implementation
  already has to make sure that the allocated memory stays
  fresh until the write is finished, since that is what
  libuv streams rely on anyway.
- The fact that a separate kind of buffer, `crypto::NodeBIO`
  was used, was confusing: These `BIO` instances are
  only used to communicate with openssl’s streams system
  otherwise, whereas this one was purely for internal
  memory management.
- The name `clear_in_` was not very helpful.

PR-URL: #17883
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>

The TLS implementation previously had two separate queues for
WriteWrap instances, one for currently active and one for
finishing writes (i.e. where the encrypted output is being written
to the underlying socket).

However, the streams implementation in Node doesn’t allow for
more than one write to be active at a time; effectively,
the only possible states were that:

- No write was active.
- The first write queue had one item, the second one was empty.
- Only the second write queue had one item, the first one was empty.

To reduce overhead and implementation complexity, remove these
queues, and instead store a single `WriteWrap` pointer and
keep track of whether its write callback should be called
on the next invocation of `InvokeQueued()` or not
(which is what being in the second queue previously effected).

PR-URL: #17883
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>

The TLS implementation previously kept a separate buffer for
incoming pieces of data, into which buffers were copied
before they were up for writing.

This removes this buffer, and replaces it with a simple list
of `uv_buf_t`s:

- The previous implementation copied all incoming data into
  that buffer, both allocating new storage and wasting time
  with copy operations. Node’s streams/net implementation
  already has to make sure that the allocated memory stays
  fresh until the write is finished, since that is what
  libuv streams rely on anyway.
- The fact that a separate kind of buffer, `crypto::NodeBIO`
  was used, was confusing: These `BIO` instances are
  only used to communicate with openssl’s streams system
  otherwise, whereas this one was purely for internal
  memory management.
- The name `clear_in_` was not very helpful.

PR-URL: #17883
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>