Skip to content

fix(binkp): apply config.hjson reloads without a restart - #721

Merged
NuSkooler merged 1 commit into
masterfrom
fix/binkp-config-hot-reload
Aug 19, 2026
Merged

fix(binkp): apply config.hjson reloads without a restart#721
NuSkooler merged 1 commit into
masterfrom
fix/binkp-config-hot-reload

Conversation

@NuSkooler

Copy link
Copy Markdown
Owner

config.hjson is watched and reloaded at runtime, but the BinkP module read most of its configuration once during startup and threaded the values into long-lived closures. The inbound listener in particular captured a BsoSpool, the local address list, and the whole binkp config block, then reused them for every connection for the life of the process. After a reload, inbound sessions kept resolving outbound directories against the old paths and networks, advertised the old local addresses, and authenticated against the old node passwords -- silently, until the BBS was restarted. The pull cycle dialed the node set as it was at boot, and the crashmail debounce window was fixed at its startup value.

Nothing here needed a new mechanism: binkp.js binds Config as a call-time indirection already, so the fix is to stop reading early.

  • _handleConnection() resolves the spool, local addresses, node/FREQ config, and tempDir per connection -- once at the top, so values can't shift mid-session. Both listeners now close over nothing but this.
  • _pullAddresses() defaults to the live config; the parameter stays for callers evaluating some other node set.
  • the crashmail debounce window is read per burst.

Two things can't be fixed by reading later, and are reconciled on the ConfigChanged event instead:

  • pullSchedule is compiled into a later.js timer, so the timer is rebuilt when the expression changes (and cleared when it is removed).
  • the inbound listener bindings -- enabled, port, address, tls.* -- are baked into a socket at listen() time. Rebinding under live sessions is not something a reload should do, so a change to those keys is reported as restart-required rather than silently ignored.

Also consolidates the three near-identical BsoSpool constructions into a single buildSpool() in binkp/util.js, whose header already asked for exactly that ("...so the three callsites can't drift").

config.hjson is watched and reloaded at runtime, but the BinkP module read
most of its configuration once during startup and threaded the values into
long-lived closures. The inbound listener in particular captured a BsoSpool,
the local address list, and the whole binkp config block, then reused them
for every connection for the life of the process. After a reload, inbound
sessions kept resolving outbound directories against the old paths and
networks, advertised the old local addresses, and authenticated against the
old node passwords -- silently, until the BBS was restarted. The pull cycle
dialed the node set as it was at boot, and the crashmail debounce window was
fixed at its startup value.

Nothing here needed a new mechanism: binkp.js binds Config as a call-time
indirection already, so the fix is to stop reading early.

  * _handleConnection() resolves the spool, local addresses, node/FREQ
    config, and tempDir per connection -- once at the top, so values can't
    shift mid-session. Both listeners now close over nothing but `this`.
  * _pullAddresses() defaults to the live config; the parameter stays for
    callers evaluating some other node set.
  * the crashmail debounce window is read per burst.

Two things can't be fixed by reading later, and are reconciled on the
ConfigChanged event instead:

  * pullSchedule is compiled into a later.js timer, so the timer is rebuilt
    when the expression changes (and cleared when it is removed).
  * the inbound listener bindings -- enabled, port, address, tls.* -- are
    baked into a socket at listen() time. Rebinding under live sessions is
    not something a reload should do, so a change to those keys is reported
    as restart-required rather than silently ignored.

Also consolidates the three near-identical BsoSpool constructions into a
single buildSpool() in binkp/util.js, whose header already asked for exactly
that ("...so the three callsites can't drift").
@NuSkooler
NuSkooler force-pushed the fix/binkp-config-hot-reload branch from 58c9033 to 2e1de11 Compare August 19, 2026 23:14
@NuSkooler
NuSkooler merged commit 571934d into master Aug 19, 2026
2 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.

1 participant