-
Notifications
You must be signed in to change notification settings - Fork 0
fix(ci): rustfmt test_support imports for PR #59 #61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
05ac02c
692d4a2
4e1c882
82f1e4f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ | |
| //! and the RTMP listener(s), then runs until a shutdown signal arrives. | ||
|
|
||
| use parking_lot::Mutex; | ||
| use std::cell::Cell; | ||
| use std::collections::{HashMap, HashSet}; | ||
| use std::sync::atomic::{AtomicBool, Ordering}; | ||
| use std::sync::{Arc, Mutex as StdMutex}; | ||
|
|
@@ -17,6 +18,22 @@ use crate::rtmp_bridge::{DbRtmpBridge, FrameInfo, FrameKind, RtmpEventHandler}; | |
| /// registered on the RTMP thread before the poll loop starts. | ||
| pub(crate) static RTMP_BRIDGE: StdMutex<Option<Arc<DbRtmpBridge>>> = StdMutex::new(None); | ||
|
|
||
| thread_local! { | ||
| static RTMP_POLL_SERVER: Cell<Option<*mut librtmp2::server::Server>> = const { | ||
| Cell::new(None) | ||
| }; | ||
| } | ||
|
|
||
| /// Pin the active RTMP server for the duration of `poll()` so publish/play | ||
| /// callbacks can resolve `remote_addr` before auth rate limiting. | ||
| pub(crate) fn set_rtmp_poll_server(server: *mut librtmp2::server::Server) { | ||
| RTMP_POLL_SERVER.with(|cell| cell.set(Some(server))); | ||
| } | ||
|
|
||
| pub(crate) fn clear_rtmp_poll_server() { | ||
| RTMP_POLL_SERVER.with(|cell| cell.set(None)); | ||
| } | ||
|
|
||
| /// How often the poll loop wakes up to service the RTMP/RTMPS listener(s). | ||
| pub(crate) const POLL_INTERVAL_MS: u64 = 50; | ||
|
|
||
|
|
@@ -64,11 +81,48 @@ where | |
| } | ||
| } | ||
|
|
||
| /// Register the client IP on the bridge before publish/play auth runs. During | ||
| /// `server.poll()` the publish/play callbacks can fire before | ||
| /// `process_server_connections` reaches `on_connect`, which would otherwise | ||
| /// skip per-IP auth-failure tracking and rate limiting. | ||
| fn ensure_conn_registered_for_auth(conn_id: u64) { | ||
| // Skip the pointer walk entirely once the normal `on_connect` pass (or an | ||
| // earlier call from this same function) has already registered the | ||
| // remote IP. Without this check every publish/play attempt on an | ||
| // already-registered connection re-ran `on_connect` and its "new | ||
| // connection" log line, which was both misleading and needless lock | ||
| // contention on the hot path. | ||
| if with_rtmp_bridge(|bridge| bridge.is_registered(conn_id)).unwrap_or(true) { | ||
| return; | ||
| } | ||
| RTMP_POLL_SERVER.with(|cell| { | ||
| let Some(server_ptr) = cell.get() else { | ||
| return; | ||
| }; | ||
| if server_ptr.is_null() { | ||
| return; | ||
| } | ||
| // SAFETY: `RTMP_POLL_SERVER` is set only on the RTMP thread for the | ||
| // duration of `server.poll()`, which exclusively owns `server`. | ||
| let server = unsafe { &*server_ptr }; | ||
| let Some(conn) = server | ||
| .connections | ||
| .iter() | ||
| .find(|c| c.conn_id == conn_id && c.client_fd >= 0) | ||
| else { | ||
| return; | ||
| }; | ||
| with_rtmp_bridge(|bridge| bridge.on_connect(conn_id, &conn.remote_addr)); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a client sends an unauthorized publish/play and then disconnects before this Useful? React with 👍 / 👎.
Comment on lines
+105
to
+115
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Confirm poll takes &mut self and inspect the publish/play callback signatures
fd -t f 'server.rs' | rg -l 'fn poll' | xargs -r rg -nP -C2 'fn poll\s*\('
rg -nP -C3 'on_publish_cb|on_play_cb|type .*PublishCb|remote_addr' $(fd -t f '.rs' | rg -i 'librtmp2|server|types' | head -50) 2>/dev/nullRepository: OpenRTMP/librtmp2-server Length of output: 2555 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- src/server.rs around the relevant code ---'
sed -n '1,140p' src/server.rs | cat -n
echo
echo '--- search for poll / thread-local server pointer usage ---'
rg -n -C 3 'set_rtmp_poll_server|RTMP_POLL_SERVER|poll\(0\)|server\.poll|on_publish_cb|on_play_cb|remote_addr' src/server.rs
echo
echo '--- all librtmp2 server method signatures available in repo ---'
rg -n -C 2 'pub fn poll|fn poll|on_publish_cb|on_play_cb|remote_addr' -g '*.rs' .Repository: OpenRTMP/librtmp2-server Length of output: 15077 Avoid creating 🤖 Prompt for AI Agents |
||
| }); | ||
| } | ||
|
|
||
| pub(crate) fn rtmp_publish_cb(conn_id: u64, app: &str, stream_key: &str) -> bool { | ||
| ensure_conn_registered_for_auth(conn_id); | ||
| with_rtmp_bridge(|b| b.authorize_publish(conn_id, app, stream_key).is_ok()).unwrap_or(false) | ||
| } | ||
|
|
||
| pub(crate) fn rtmp_play_cb(conn_id: u64, app: &str, play_key: &str) -> bool { | ||
| ensure_conn_registered_for_auth(conn_id); | ||
| with_rtmp_bridge(|b| b.authorize_play(conn_id, app, play_key).is_ok()).unwrap_or(false) | ||
| } | ||
|
|
||
|
|
@@ -543,7 +597,10 @@ impl ServerApp { | |
| break; | ||
| } | ||
|
|
||
| if let Err(e) = server.poll(0) { | ||
| set_rtmp_poll_server(&mut server); | ||
| let poll_result = server.poll(0); | ||
| clear_rtmp_poll_server(); | ||
| if let Err(e) = poll_result { | ||
| crate::log_warn!("RTMP polling stopped: {e}"); | ||
| break; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a publish/play callback fires from
server.poll(0),librtmp2::Server::poll(&mut self)is still inside its connection-processing loop, soself.connectionsis already mutably borrowed while a connection is being parsed. Dereferencing the thread-local raw pointer here creates a shared&Serverand iterates the sameconnectionsvector reentrantly, which is undefined behavior in Rust even on the same thread; under live RTMP publish/play traffic this can miscompile or crash instead of just looking up the remote address. The callback path needs to avoid readingServerthrough a raw alias whilepollowns&mut self(for example by having the library pass the remote address or by maintaining a separate pre-populated conn-id map).Useful? React with 👍 / 👎.