Skip to content

Fix relay scheduler cleanup between runs - #46

Merged
sjanusz-r7 merged 2 commits into
rapid7:masterfrom
mjatin2007:jatin/fix-relay-manager-cleanup
Oct 14, 2026
Merged

sjanusz-r7 merged 2 commits into
rapid7:masterfrom
mjatin2007:jatin/fix-relay-manager-cleanup

Conversation

@mjatin2007

@mjatin2007 mjatin2007 commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Description

RelayManager currently calls FiberScheduler#reset! whenever its worker exits. The scheduler is closed first, but reset! immediately allocates a new signaling pipe and leaves both descriptors attached to the idle manager until garbage collection. In the Metasploit MSSQL connection path, repeated logins can therefore retain descriptors after the TCP socket, channel socket pair, and relay worker have closed; the increase per login depends on whether the connection path creates one or more idle managers.

This change leaves the signaling pipe unallocated after reset and recreates it lazily when new work is scheduled. It starts an idle worker behind a gate before accepting the first relay, then serializes relay submission with the worker's pending-work and teardown decision. An external submission waits once teardown commits, while a callback already running on the worker can submit follow-up work without blocking. FiberScheduler#close preserves the first work error, drains remaining accepted work, and releases the pipe in an ensure path.

Related to rapid7/metasploit-framework#22002.

Verification

  • Full rex-core suite: 127 examples, 0 failures. The final 18-job matrix passed on Ruby 3.2, 3.3, and 3.4 across Ubuntu 22.04, Ubuntu 24.04, ubuntu-latest, Windows Server 2022, Windows Server 2025, and Intel macOS 15. The suite also passed natively on Apple Silicon macOS 26.6.1 with Ruby 3.3.12.
  • Native Apple Silicon macOS direct control/candidate test with garbage collection disabled: ten fresh managers on pre-fix commit 349fe64 increased /dev/fd from 7 to 27; this candidate remained at 7. One hundred sequential reuses and 50 four-thread submission groups also remained at their candidate baselines.
  • Live SQL Server 2022 with client-requested TDS encryption: five wrong-password attempts increased released rex-core 0.1.36 from 7 to 17 descriptors, followed by five successful-login cleanups to 27; this candidate remained at 7 throughout. Every connection negotiated TLS 1.2 with ECDHE-RSA-AES128-GCM-SHA256, and successful sessions reported encrypt_option = TRUE.
  • Live SQL Server 2022 with server-required encryption and the client initially setting TDSENCRYPTION=false: the server ENCRYPT_REQ reconnect path changed the observed client state to encrypted. Five wrong-password attempts increased the 0.1.36 control from 7 to 27 descriptors, followed by five successful-login cleanups to 47; this candidate remained at 7 throughout.
  • Gem build: succeeded.
  • Frozen Metasploit PR #22002 channel plus this candidate, with garbage collection disabled: 50/50 post-cleanup descriptor measurements matched their scenario baselines across successful login without a retained session, failed login, login exception, retained-session cleanup, and disconnect/reconnect.
  • Relay stress: 600 cycles covering sequential reuse, four-thread submission, and callback-scheduled follow-up work; every group returned to its descriptor baseline.
  • Expanded Metasploit login-scanner, MsTds, MSSQL, and session scope with the candidate: 2,556 examples, 0 failures.

Both live SQL comparisons ran with garbage collection disabled for the released control and candidate. They used SQL Server 2022 in a Linux container and a Linux arm64 Ruby 3.3.8 client. The native macOS resource test exercised rex-core directly. Windows validation reported here consists of the completed rex-core suite, not a process-handle probe. No live SQL test ran from macOS or Windows. The self-signed SQL Server certificate validates TLS negotiation and cleanup, not certificate-authentication behavior. I have not reproduced the reporter's exact Metasploit/macOS environment, exercised NTLM or Kerberos, run the whole Metasploit suite, or established that this mechanism accounts for every descriptor in the reported 12-to-24 change.

Reviewer Notes

  • add_relay releases a newly created worker's gate in an ensure, including when lazy pipe creation or scheduling raises; the worker can therefore finish teardown and the manager remains reusable.
  • Scheduled blocks and ready fibers are removed under the scheduler mutex and resumed after that mutex is released, so callbacks do not execute while it is held.
  • unblock returning false when the signaling pipe is nil relies on the invariant that nil means the scheduler is reset/closed with no live blocked fiber. Close intentionally drains accepted work before propagating its first error; work that continually reschedules itself can therefore delay close.
  • If the main scheduler run and the automatic close drain both raise, Ruby's ensure semantics propagate the close-drain error; FiberScheduler#close preserves the first error observed within that drain, rather than the earlier superseded worker error.
  • The second commit only makes the existing blocking-count spec wait for the scheduler's actual blocked state before unblocking it. It does not change production code.

AI Usage Disclosure

OpenAI Codex coordinated source analysis, drafted code and tests, and ran the validation. The contributor reviewed the public changes before publication.

Signed-off-by: mjatin2007 <mjatin2007@users.noreply.github.com>
mjatin2007 added a commit to mjatin2007/rex-core that referenced this pull request Oct 13, 2026
Signed-off-by: mjatin2007 <mjatin2007@users.noreply.github.com>
Signed-off-by: mjatin2007 <mjatin2007@users.noreply.github.com>
@sjanusz-r7

Copy link
Copy Markdown
Contributor

Thanks for this! This PR paired with the linked Metasploit Framework PR resulted in no leaking sockets and no leaking file descriptors, tested using lsof and Dir.children("/dev/fd").count.

I can merge this in, rex-core would then be bumped automatically. You can then bump the gem in the original framework PR, and we can get the original issue closed. Thank you!

@sjanusz-r7
sjanusz-r7 merged commit 7e07e8d into rapid7:master Oct 14, 2026
20 checks passed
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