Skip to content

tls: fix writeQueueSize prop, long write timeouts

Rodrigo Muino Tomonari requested to merge github/fork/apapirovski/patch-http-keep-alive-timeout into master

This PR addresses a long-standing issue of _handle.writeQueueSize in TLS not representing the actual write queue (https://github.com/nodejs/node/issues/15005), as well as the fact that long-lasting writes on both net & tls (http & https) will currently timeout if a socket timeout is set (https://github.com/nodejs/node/issues/15082). The latter has been reported as being in relation to the keep alive timeout but in reality it's just a broad issue that is the most obvious with keep-alive since it's much shorter at 5s (vs 120s).

  • Make writeQueueSize represent the actual size of the write queue within the TLS socket.
  • Add tls test to confirm that bufferSize works as expected.

Second commit:

  • Add getWriteQueueSize on TLSWrap which updates and returns queue size (net & tls).
  • Make _onTimeout check whether an active write is ongoing and if so, call _unrefTimer rather than emitting a timeout event.
  • Add http & https test that checks whether long-lasting (but active) writes timeout or can finish writing as expected.

Haven't worked in C++ in a very, very long time so would definitely appreciate any and all feedback there.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

http, https, net, test, tls

Merge request reports