Skip to content

fix(transport): GC TLS processes after handshake on OTP 28 - #240

Merged
zmstone merged 1 commit into
emqx:masterfrom
terry-xiaoyu:fix/otp28-gc-after-handshake
Nov 10, 2026
Merged

zmstone merged 1 commit into
emqx:masterfrom
terry-xiaoyu:fix/otp28-gc-after-handshake

Conversation

@terry-xiaoyu

@terry-xiaoyu terry-xiaoyu commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Summary

gc_after_handshake silently stopped collecting TLS processes on OTP 28. OTP 28 changed the sslsocket tuple from 3 to 8 elements; the TLS worker pids moved to the connection_handler and payload_sender fields, so the existing gc/1 clauses matched only OTP <= 27 sockets and fell through to a no-op.

Add a clause for the 8-element sslsocket shape and share pid de-duplication with the legacy 3-element shapes. This restores the OTP <= 27 behaviour on OTP 28. DTLS is covered by the same path (both fields hold the same pid there).

Only the TLS worker processes are collected. The per-connection tls_dyn_connection_sup is not: OTP hibernates static supervisors on its own (during init on OTP 24-27, after the default 1 s hibernate_after on OTP 28), so collecting it at handshake would only save memory for the first second on OTP 28, and never on older releases.

The option is extracted from ssl_options before ssl:handshake/2,3, so nothing here changes the OTP option contract.

Backport

The commit applies to the 5.17.x line and is needed there for 5.17.10.

Testing

New Common Test cases in esockd_transport_SUITE drive a real TLS/DTLS handshake and assert major-GC trace events on the expected TLS worker processes:

  • option enabled: the connection handler and the sender are collected;
  • option disabled: neither is collected;
  • #proxy_socket{} recursion;
  • DTLS: both worker fields are the same pid and the global dtls_connection_sup is not collected;
  • OTP 28 listen sockets (undefined pids) are a no-op.

The cases fail on the unfixed source with missing_gc_major and pass with the change on OTP 28. The full transport suite and rebar3 xref pass.

@zmstone zmstone left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mind this change: emqx/otp#129

@terry-xiaoyu
terry-xiaoyu marked this pull request as ready for review November 10, 2026 08:14
Comment thread src/esockd_transport.erl Resolved
@terry-xiaoyu
terry-xiaoyu force-pushed the fix/otp28-gc-after-handshake branch from 80ad752 to 9dbe7b1 Compare November 10, 2026 09:40
@terry-xiaoyu

Copy link
Copy Markdown
Contributor Author

mind this change: emqx/otp#129

verified on the fork OTP that has it (OTP-28.4.1-11, the builder image pinned by dev-61). With a real server-side handshake through esockd, under both ssl.connection_tree_owner=supervisor and = caller, gc/1 collects the connection handler and the sender. #129 only changes who starts the connection tree and unlinks the caller; the receiver-supervisor link and the supervisor's initial call are unchanged, so nothing in gc/1 depends on it.

Comment thread src/esockd_transport.erl Resolved
Comment thread src/esockd_transport.erl Resolved
Comment thread test/esockd_transport_SUITE.erl Resolved
OTP 28 changed the sslsocket tuple from 3 to 8 elements. The TLS worker
pids now live in the connection_handler and payload_sender fields, so the
existing gc/1 clauses matched only OTP<=27 sockets and silently skipped
the post-handshake GC on OTP 28.

Add a clause for the 8-element shape and share pid de-duplication across
the old and new shapes.
@terry-xiaoyu
terry-xiaoyu force-pushed the fix/otp28-gc-after-handshake branch from 9dbe7b1 to 4990d02 Compare November 10, 2026 11:48
@zmstone
zmstone merged commit 051b65a into emqx:master Nov 10, 2026
8 checks passed
@zmstone zmstone mentioned this pull request Nov 10, 2026
2 of 3 tasks
@terry-xiaoyu
terry-xiaoyu deleted the fix/otp28-gc-after-handshake branch November 10, 2026 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants