Skip to content

Use socket pairs for remaining scheduler control fds on Windows (fixes #5245) - #5290

Merged
toots merged 1 commit into
savonet:mainfrom
cdgraff:fix/output-harbor-windows-select-leak
Jul 31, 2026
Merged

Use socket pairs for remaining scheduler control fds on Windows (fixes #5245)#5290
toots merged 1 commit into
savonet:mainfrom
cdgraff:fix/output-harbor-windows-select-leak

Conversation

@cdgraff

@cdgraff cdgraff commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Fixes #5245.

Root cause

The leak is Windows-only because it lives in OCaml's Unix.select emulation, not in output.harbor itself. On Unix, duppy uses the poll() C stub, which is why the leak never reproduced on mac/linux.

OCaml's select_win32.c has two paths:

  • A plain winsock select() fast path, taken only when every fd in the call is a socket ("only sockets to select on, using classic select").
  • Otherwise, a worker-thread emulation (winworker.c + WSAEventSelect + per-call event handles) that leaks native memory on every call — invisible to the OCaml GC, which matches the flat process_managed_memory in the issue logs.

Two pipes were permanently registered in the Tutils scheduler, so every scheduler iteration took the leaking path:

  1. harbor.ml — the accept-loop control fd (Unix.pipe)
  2. duppy.ml — the Duppy.Monad.Mutex.Factory wake pipe, created unconditionally when server.ml loads

This also explains the observed behavior across versions: leak rate scales with scheduler wake rate. On 2.4.5 the scheduler only churns while listeners are connected (~45 MB/min with one client, stable without). On main, the rewritten harbor output wakes the write task every frame, so memory grows even with no listeners, and faster (~280 MB/min).

Fix

Convert both remaining pipes to Unix_utils.socketpair, following the same conversion already done for the scheduler wake fd, Duppy.Async, and the harbor output write task. With sockets only in the set, Windows select always takes the leak-free fast path.

Notes

  • process_handler pipes still enter the scheduler while an external process runs, so scripts using process.run on Windows would temporarily re-enter the emulation path. Left out of scope here.
  • Independent upstream issue worth reporting to ocaml/ocaml: in socket_poll (select_win32.c), any mid-setup error skips the entire cleanup loop, leaking all CreateEvent handles and leaving WSAEventSelect associations on the sockets.

Validation

We can reproduce the leak reliably on Windows 11 (25+ machines, physical and virtual). Once CI produces the Windows artifact for this branch we'll run the reproduction script from #5245 and report memory behavior with and without connected listeners.

…net#5245)

On Windows, OCaml's Unix.select only takes its plain winsock fast path
when every fd in the call is a socket. A single pipe forces the whole
call through the worker-thread/WSAEventSelect emulation, which leaks
native memory on every call. The harbor accept-loop control pipe and
the Duppy.Monad.Mutex wake pipe were the last two non-socket fds
permanently registered in the scheduler, so every scheduler iteration
took the leaking path. Leak rate scales with scheduler wake rate,
which is why memory grew fastest with connected listeners.
@toots

toots commented Jul 31, 2026

Copy link
Copy Markdown
Member

Great find thanks! I have confirmed and reported the issue with the OCaml maintainers: ocaml/ocaml#14975

@toots
toots merged commit 1b05d07 into savonet:main Jul 31, 2026
42 checks passed
siran added a commit to siran/radio that referenced this pull request Aug 24, 2026
Unix.select on Windows takes a leaking WSAEventSelect path when any descriptor in the set is not a socket, and liquidsoap kept two pipes there permanently. savonet/liquidsoap#5290, merged 1b05d07. Section 6 stays as the record of what was ruled out and how.
siran added a commit to siran/radio that referenced this pull request Aug 24, 2026
savonet/liquidsoap#5290 (1b05d07) fixes the Unix.select leak but went to main only; it was never backported to v2.4.x-latest, so the 2.4.6 rolling build does not contain it. Checked: 1b05d07...469ffee diverges, behind_by 446. The only prebuilt Windows binary with the fix is 2.5.0 rolling.

Two changes, and the documented 2.5 breaking list touches nothing else here - no fallback, rotate, random, cross, crossfade, video canvas or removed setting anywhere in this file.

1. switch lost track_sensitive and replay_metadata as parallel arguments; they are per-source methods now. BOTH branches carry them, because the old arguments covered the whole operator and a branch left bare would take the defaults and re-announce metadata on the way back from a pause - the exact fault the comment above that call was written for.

2. settings.source.composition.max_fade := 0. 2.5 fades when a switch cuts into a playing track, up to a second. That switch IS the pause, so every pause and unpause would have gained a fade the listener page's record animation was never timed against.

Verified on this machine before deploying. Minimal script that runs on both builds, 5 rounds x 2000 requests: 2.4.5 grew +7.3 +7.9 +7.6 +7.9 +7.9 MB per round, linear and never stopping; 2.5.0 grew +27.0 then +2.3 +0.3 +0.0 +0.0 and stopped dead. 2.4.5 also took 22-23s per round against 2.5.0's 9s, which is the slow worker-thread emulation path the diagnosis blames.

Full station on 2.5.0 in a lab on spare ports: audio encoding, all four endpoints, telnet, the announcer, and pause engaging and releasing. Leak shape +15.3 -0.5 -1.9 +3.4 across 4x1500 requests - two negative rounds, which a leak cannot do.
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.

Memory leak in output.harbor when clients are actively listening

2 participants