Skip to content

Commit c796452

Browse files
committed
fix(bootstrap): bind HTTP and sync listeners before any accept loop starts
Previously HTTP and sync sockets were bound lazily inside their own spawned tasks, so a port conflict on either surfaced late (or, for HTTP, only as a task failure) after the node had already started serving other protocols. Move both binds into bind_listeners alongside the native and pgwire sockets so every protocol's port conflict is boot-fatal before anything is exposed, then hand the pre-bound sockets to serve_sync_listener/run via new BoundListeners/ProtocolListeners fields. Also introduce DEFAULT_SYNC_PORT as the single source of truth shared between ServerConfig and SyncListenerConfig::default(), fix the wasm docs example to reference the real default sync port, and give the shutdown/crash-harness integration tests their own unique sync ports so concurrent server processes never collide on the default.
1 parent 0c8cefc commit c796452

14 files changed

Lines changed: 211 additions & 107 deletions

File tree

docs/getting-started.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,10 @@ resp = true
237237
ilp = false # Example: disable TLS for ILP ingest
238238
```
239239

240+
Every listener binds during startup, before the server accepts any
241+
connection. If a configured port is already in use, startup fails with that
242+
port named — the server never comes up missing a protocol.
243+
240244
**Server settings:**
241245

242246
| Config field | Environment variable | Default |

docs/wasm.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ const db = new NodeDbLite();
173173

174174
// Configure sync to Origin
175175
await db.sync_config({
176-
server_url: "ws://localhost:6433",
176+
server_url: "ws://localhost:9090",
177177
auth_token: "your-token",
178178
auto_sync: true,
179179
sync_interval_ms: 5000,

nodedb/src/bootstrap/listeners.rs

Lines changed: 55 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use std::sync::Arc;
66

77
use anyhow::Context;
8+
use tokio::net::TcpListener;
89
use tracing::info;
910

1011
use crate::ServerConfig;
@@ -17,9 +18,14 @@ use crate::control::shutdown::ShutdownBus;
1718
use crate::control::startup::StartupGate;
1819
use crate::control::state::SharedState;
1920

20-
/// The three protocol listeners passed to [`spawn_protocol_listeners`].
21+
/// The pre-bound protocol listeners passed to [`spawn_protocol_listeners`].
22+
///
23+
/// Every socket here is already bound by [`bind_listeners`], so spawning
24+
/// cannot fail on a port conflict.
2125
pub struct ProtocolListeners {
2226
pub pg_listener: PgListener,
27+
pub http_listener: TcpListener,
28+
pub sync_listener: TcpListener,
2329
pub ilp_listener: Option<IlpListener>,
2430
pub resp_listener: Option<RespListener>,
2531
}
@@ -36,20 +42,21 @@ pub struct ListenerInfra {
3642
/// The native listener is not spawned here — it is run on the main task
3743
/// by the caller after this returns.
3844
///
39-
/// Returns `Err` if the sync WebSocket listener fails to bind — that
40-
/// failure is boot-fatal rather than a silent warning, since a server
41-
/// that comes up without a reachable sync listener leaves NodeDB-Lite
42-
/// clients unable to connect with no indication anything is wrong.
45+
/// Infallible by construction: every socket was bound by [`bind_listeners`]
46+
/// before this point, so a port conflict has already aborted boot while
47+
/// nothing was exposed. Nothing here may silently swallow a bind failure.
4348
pub async fn spawn_protocol_listeners(
4449
listeners: ProtocolListeners,
4550
shared: Arc<SharedState>,
4651
config: &ServerConfig,
4752
infra: ListenerInfra,
4853
base_acceptor: Option<tokio_rustls::TlsAcceptor>,
4954
cluster_handle: &Option<Arc<ClusterHandle>>,
50-
) -> anyhow::Result<()> {
55+
) {
5156
let ProtocolListeners {
5257
pg_listener,
58+
http_listener,
59+
sync_listener,
5360
ilp_listener,
5461
resp_listener,
5562
} = listeners;
@@ -90,10 +97,9 @@ pub async fn spawn_protocol_listeners(
9097
}
9198
});
9299

93-
// HTTP API server.
100+
// HTTP API server (on the socket bound by `bind_listeners`).
94101
let shared_http = Arc::clone(&shared);
95102
let http_auth_mode = config.auth.mode.clone();
96-
let http_listen = config.http_addr();
97103
let http_tls = if http_tls_enabled {
98104
config.server.tls.clone()
99105
} else {
@@ -102,7 +108,7 @@ pub async fn spawn_protocol_listeners(
102108
let bus_http = shutdown_bus.clone();
103109
tokio::spawn(async move {
104110
if let Err(e) = crate::control::server::http::server::run(
105-
http_listen,
111+
http_listener,
106112
shared_http,
107113
http_auth_mode,
108114
http_tls.as_ref(),
@@ -162,51 +168,59 @@ pub async fn spawn_protocol_listeners(
162168
});
163169
}
164170

165-
// Sync WebSocket listener for NodeDB-Lite clients.
171+
// Sync WebSocket listener for NodeDB-Lite clients (socket already bound).
166172
let sync_config = crate::control::server::sync::listener::SyncListenerConfig {
167173
listen_addr: config.sync_addr(),
168174
..Default::default()
169175
};
170-
match crate::control::server::sync::listener::start_sync_listener(
176+
let sync_state = crate::control::server::sync::listener::serve_sync_listener(
177+
sync_listener,
171178
sync_config,
172179
Some(Arc::clone(&shared)),
173180
)
174-
.await
175-
{
176-
Ok(sync_state) => {
177-
info!(
178-
addr = %sync_state.config.listen_addr,
179-
max_sessions = sync_state.config.max_sessions,
180-
"sync WebSocket listener started"
181-
);
182-
}
183-
Err(e) => {
184-
return Err(e).context("sync listener failed to bind");
185-
}
186-
}
181+
.await;
182+
info!(
183+
addr = %sync_state.config.listen_addr,
184+
max_sessions = sync_state.config.max_sessions,
185+
"sync WebSocket listener started"
186+
);
187187

188188
// Signal readiness to systemd and cluster lifecycle.
189189
if let Some(handle) = cluster_handle {
190190
let nodes = handle.topology.read().map(|t| t.node_count()).unwrap_or(1);
191191
handle.lifecycle.to_ready(nodes);
192192
}
193193
nodedb_cluster::readiness::notify_ready();
194+
}
194195

195-
Ok(())
196+
/// Every protocol socket, bound before any accept loop starts.
197+
pub struct BoundListeners {
198+
pub native: Listener,
199+
pub pgwire: PgListener,
200+
pub http: TcpListener,
201+
pub sync: TcpListener,
202+
pub ilp: Option<IlpListener>,
203+
pub resp: Option<RespListener>,
196204
}
197205

198206
/// Bind all protocol listeners to their configured addresses.
199-
pub async fn bind_listeners(
200-
config: &ServerConfig,
201-
) -> anyhow::Result<(
202-
Listener,
203-
PgListener,
204-
Option<IlpListener>,
205-
Option<RespListener>,
206-
)> {
207+
///
208+
/// This is the single fail-fast point for listener setup: it runs before the
209+
/// node waits on cluster readiness and before any accept loop is spawned, so
210+
/// a port conflict on *any* protocol — including HTTP and sync, which serve
211+
/// from detached tasks — aborts boot while nothing is exposed yet. Never
212+
/// move a bind out of here into a spawned task; that is how a server ends up
213+
/// running for days missing a listener behind one warning line.
214+
pub async fn bind_listeners(config: &ServerConfig) -> anyhow::Result<BoundListeners> {
207215
let native = crate::control::server::listener::Listener::bind(config.native_addr()).await?;
208216
let pgwire =
209217
crate::control::server::pgwire::listener::PgListener::bind(config.pgwire_addr()).await?;
218+
let http = TcpListener::bind(config.http_addr())
219+
.await
220+
.with_context(|| format!("bind HTTP API listener to {}", config.http_addr()))?;
221+
let sync = crate::control::server::sync::listener::bind_sync_listener(config.sync_addr())
222+
.await
223+
.context("sync listener failed to bind")?;
210224
let ilp = if let Some(ilp_addr) = config.ilp_addr() {
211225
Some(crate::control::server::ilp_listener::IlpListener::bind(ilp_addr).await?)
212226
} else {
@@ -217,5 +231,12 @@ pub async fn bind_listeners(
217231
} else {
218232
None
219233
};
220-
Ok((native, pgwire, ilp, resp))
234+
Ok(BoundListeners {
235+
native,
236+
pgwire,
237+
http,
238+
sync,
239+
ilp,
240+
resp,
241+
})
221242
}

nodedb/src/config/server/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ pub use observability::{
2525
ObservabilityConfig, OtlpConfig, OtlpExportConfig, OtlpReceiverConfig, PromqlConfig,
2626
apply_observability_env, validate_feature_availability,
2727
};
28-
pub use ports::PortsConfig;
28+
pub use ports::{DEFAULT_SYNC_PORT, PortsConfig};
2929
pub use retention::RetentionSettings;
3030
pub use scheduler::{CronTimezone, SchedulerConfig};
3131
pub use section::ServerSection;

nodedb/src/config/server/ports.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,13 @@
44
55
use serde::{Deserialize, Serialize};
66

7+
/// Default sync WebSocket port.
8+
///
9+
/// Exported because the sync listener's own `SyncListenerConfig::default()`
10+
/// must agree with the config default — one constant, not two literals that
11+
/// can drift apart.
12+
pub const DEFAULT_SYNC_PORT: u16 = 9090;
13+
714
/// Port configuration for all protocol listeners.
815
///
916
/// Always-on protocols have a default port. Optional protocols (RESP, ILP)
@@ -54,7 +61,7 @@ fn default_http_port() -> u16 {
5461
6480
5562
}
5663
fn default_sync_port() -> u16 {
57-
9090
64+
DEFAULT_SYNC_PORT
5865
}
5966

6067
#[cfg(test)]

nodedb/src/control/server/http/server.rs

Lines changed: 20 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@
3737
//! middleware (auth, startup-gate, tracing) must apply uniformly across
3838
//! versions.
3939
40-
use std::net::SocketAddr;
4140
use std::sync::Arc;
4241

4342
use axum::Router;
@@ -213,61 +212,26 @@ async fn startup_gate_middleware(
213212

214213
/// Start the HTTP API server from an already-bound [`tokio::net::TcpListener`].
215214
///
216-
/// Useful in tests where an ephemeral-port listener is bound before the server
217-
/// task is spawned, making the port available to the test without a race.
215+
/// Alias of [`run`], kept for call sites (tests, harnesses) that bind an
216+
/// ephemeral-port listener first so the port is known without a race.
218217
pub async fn run_with_listener(
219218
listener: tokio::net::TcpListener,
220219
shared: Arc<SharedState>,
221220
auth_mode: AuthMode,
222221
tls_settings: Option<&crate::config::server::TlsSettings>,
223222
bus: crate::control::shutdown::ShutdownBus,
224223
) -> crate::Result<()> {
225-
if tls_settings.is_some() {
226-
return Err(crate::Error::Config {
227-
detail: "run_with_listener does not support TLS; use run() instead".into(),
228-
});
229-
}
230-
let drain_guard = bus.register_task(
231-
crate::control::shutdown::ShutdownPhase::DrainingListeners,
232-
"http",
233-
None,
234-
);
235-
let mut shutdown_rx = bus.handle().flat_watch().raw_receiver();
236-
237-
let query_ctx = Arc::new(crate::control::planner::context::QueryContext::for_state(
238-
&shared,
239-
));
240-
let state = AppState {
241-
shared,
242-
auth_mode,
243-
query_ctx,
244-
};
245-
let router = build_router(state);
246-
let local_addr = listener.local_addr()?;
247-
info!(%local_addr, "HTTP API server listening (pre-bound listener)");
248-
// `with_connect_info` is required so routes that take
249-
// `ConnectInfo<SocketAddr>` (e.g. `/api/auth/session` for the
250-
// fingerprint-bound session handle path) resolve the peer address.
251-
// Without it, axum rejects those requests with 500.
252-
axum::serve(
253-
listener,
254-
router.into_make_service_with_connect_info::<std::net::SocketAddr>(),
255-
)
256-
.with_graceful_shutdown(async move {
257-
let _ = shutdown_rx.changed().await;
258-
})
259-
.await
260-
.map_err(crate::Error::Io)?;
261-
drain_guard.report_drained();
262-
Ok(())
224+
run(listener, shared, auth_mode, tls_settings, bus).await
263225
}
264226

265-
/// Start the HTTP API server (plain HTTP or HTTPS).
227+
/// Start the HTTP API server (plain HTTP or HTTPS) on a pre-bound listener.
266228
///
267-
/// If `tls_settings` is provided, serves HTTPS via axum-server + rustls.
268-
/// Otherwise serves plain HTTP via axum::serve.
229+
/// The socket is bound by `bootstrap::listeners::bind_listeners` before any
230+
/// accept loop starts, so a port conflict is boot-fatal rather than an error
231+
/// logged from inside an already-detached task. If `tls_settings` is provided,
232+
/// serves HTTPS via axum-server + rustls; otherwise plain HTTP via axum::serve.
269233
pub async fn run(
270-
listen: SocketAddr,
234+
listener: tokio::net::TcpListener,
271235
shared: Arc<SharedState>,
272236
auth_mode: AuthMode,
273237
tls_settings: Option<&crate::config::server::TlsSettings>,
@@ -289,17 +253,18 @@ pub async fn run(
289253
query_ctx,
290254
};
291255
let router = build_router(state);
256+
let local_addr = listener.local_addr()?;
292257

293258
if let Some(tls) = tls_settings {
294-
// HTTPS via axum-server + rustls.
259+
// HTTPS via axum-server + rustls, on the pre-bound socket.
295260
let rustls_config =
296261
axum_server::tls_rustls::RustlsConfig::from_pem_file(&tls.cert_path, &tls.key_path)
297262
.await
298263
.map_err(|e| crate::Error::Config {
299264
detail: format!("HTTP TLS config error: {e}"),
300265
})?;
301266

302-
info!(%listen, tls = true, "HTTPS API server listening");
267+
info!(%local_addr, tls = true, "HTTPS API server listening");
303268

304269
let handle = axum_server::Handle::new();
305270
let shutdown_handle = handle.clone();
@@ -308,17 +273,22 @@ pub async fn run(
308273
shutdown_handle.graceful_shutdown(Some(std::time::Duration::from_secs(5)));
309274
});
310275

311-
axum_server::bind_rustls(listen, rustls_config)
276+
// `into_std` keeps the socket in nonblocking mode, which is what
277+
// axum-server needs to hand it back to Tokio.
278+
axum_server::from_tcp_rustls(listener.into_std()?, rustls_config)
279+
.map_err(crate::Error::Io)?
312280
.handle(handle)
313281
.serve(router.into_make_service_with_connect_info::<std::net::SocketAddr>())
314282
.await
315283
.map_err(crate::Error::Io)?;
316284
} else {
317285
// Plain HTTP.
318-
let listener = tokio::net::TcpListener::bind(listen).await?;
319-
let local_addr = listener.local_addr()?;
320286
info!(%local_addr, "HTTP API server listening");
321287

288+
// `with_connect_info` is required so routes that take
289+
// `ConnectInfo<SocketAddr>` (e.g. `/api/auth/session` for the
290+
// fingerprint-bound session handle path) resolve the peer address.
291+
// Without it, axum rejects those requests with 500.
322292
axum::serve(
323293
listener,
324294
router.into_make_service_with_connect_info::<std::net::SocketAddr>(),

0 commit comments

Comments
 (0)