Skip to content

Commit aa458f1

Browse files
zmerpclaude
andauthored
feat(sockets, server_core): unified socket abstraction (#3321)
* refactor(server_core, server_openvr): remove alvr_restart - Remove alvr_restart() from the C API and restart() from ServerCoreContext; RestartPending now handled identically to ShutdownPending in server_openvr - Fix latent bug: restart path missing lifecycle_state=ShuttingDown and early return, causing fall-through into StartStream handshake Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix/chore(server_core, packets): misc connection_pipeline cleanups - Internalize HandGestureManager into tracking_loop as a local variable, removing the Arc<Mutex<>> parameter - Move disconnect_notif and tracking_manager resets to be grouped together after the handshake completes, next to bitrate_manager - Remove redundant braces around enable_on_connect_script block - Remove stale debug log, update log messages - Note that server_ip in ClientConnectionResult is currently unused Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(sockets): add SocketConnection abstraction Wraps ProtoControlSocket + StreamSocket into a unified SocketConnection that hides the handshake protocol. Adds connect_to_client, listen_to_server and send_restart_signal free functions. StreamSocketConfig consolidates buffer config into a single field. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(server_core): use SocketConnection in connection_pipeline Replace manual handshake (proto_socket.split + StreamSocketBuilder + separate stream setup) with SocketConnection.from_client_connection. Move connection_result recv into alvr_sockets::connect_to_client. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 17448c1 commit aa458f1

10 files changed

Lines changed: 261 additions & 139 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

alvr/packets/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ impl VideoStreamingCapabilities {
6565
pub struct ConnectionAcceptedInfo {
6666
pub client_protocol_id: u64,
6767
pub platform_string: String,
68-
pub server_ip: IpAddr,
68+
pub server_ip: IpAddr, // must be unused for now
6969
pub streaming_capabilities: Option<VideoStreamingCapabilities>,
7070
}
7171

alvr/server_core/src/c_api.rs

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -562,13 +562,6 @@ pub unsafe extern "C" fn alvr_duration_until_next_vsync(out_ns: *mut u64) -> boo
562562
}
563563
}
564564

565-
#[unsafe(no_mangle)]
566-
pub extern "C" fn alvr_restart() {
567-
if let Some(context) = SERVER_CORE_CONTEXT.write().take() {
568-
context.restart();
569-
}
570-
}
571-
572565
#[unsafe(no_mangle)]
573566
pub extern "C" fn alvr_shutdown() {
574567
SERVER_CORE_CONTEXT.write().take();

alvr/server_core/src/connection.rs

Lines changed: 62 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ use crate::{
22
ConnectionContext, FILESYSTEM_LAYOUT, SESSION_MANAGER, ServerCoreEvent,
33
ServerNegotiatedStreamingConfig,
44
bitrate::BitrateManager,
5-
hand_gestures::HandGestureManager,
65
input_mapping::ButtonMappingManager,
76
sockets::WelcomeSocket,
87
statistics::StatisticsManager,
@@ -30,8 +29,8 @@ use alvr_session::{
3029
SocketProtocol, SteamvrHmdInitConfig,
3130
};
3231
use alvr_sockets::{
33-
CONTROL_PORT, KEEPALIVE_INTERVAL, KEEPALIVE_TIMEOUT, PeerType, ProtoControlSocket,
34-
StreamSocketBuilder, WIRED_CLIENT_HOSTNAME,
32+
CONTROL_PORT, KEEPALIVE_INTERVAL, KEEPALIVE_TIMEOUT, ProtoControlSocket, SocketConnection,
33+
StreamSocketConfig, WIRED_CLIENT_HOSTNAME,
3534
};
3635
use std::{
3736
collections::{HashMap, hash_map::DefaultHasher},
@@ -485,9 +484,9 @@ fn try_connect(
485484
) -> ConResult {
486485
dbg_connection!("try_connect: Finding client and creating control socket");
487486

488-
let (proto_socket, client_ip) = ProtoControlSocket::connect_to(
487+
let (socket, client_ip, connection_result) = alvr_sockets::connect_to_client(
488+
client_ips.keys().cloned().collect(),
489489
Duration::from_secs(1),
490-
PeerType::AnyClient(client_ips.keys().cloned().collect()),
491490
)?;
492491

493492
let Some(client_hostname) = client_ips.remove(&client_ip) else {
@@ -502,7 +501,8 @@ fn try_connect(
502501
if let Err(e) = connection_pipeline(
503502
Arc::clone(&ctx),
504503
lifecycle_state,
505-
proto_socket,
504+
socket,
505+
connection_result,
506506
client_hostname.clone(),
507507
client_ip,
508508
) {
@@ -530,7 +530,8 @@ fn try_connect(
530530
fn connection_pipeline(
531531
ctx: Arc<ConnectionContext>,
532532
lifecycle_state: Arc<RwLock<LifecycleState>>,
533-
mut proto_socket: ProtoControlSocket,
533+
socket: ProtoControlSocket,
534+
connection_result: ClientConnectionResult,
534535
client_hostname: String,
535536
client_ip: IpAddr,
536537
) -> ConResult {
@@ -551,21 +552,6 @@ fn connection_pipeline(
551552
ClientConnectionsAction::UpdateCurrentIp(Some(client_ip)),
552553
);
553554

554-
let disconnect_notif = Arc::new(Condvar::new());
555-
556-
dbg_connection!("connection_pipeline: Getting client status packet");
557-
let connection_result = match proto_socket.recv(HANDSHAKE_ACTION_TIMEOUT) {
558-
Ok(r) => r,
559-
Err(ConnectionError::TryAgain(e)) => {
560-
debug!(
561-
"Failed to recive client connection packet. This is normal for USB connection.\n{e}"
562-
);
563-
564-
return Ok(());
565-
}
566-
Err(e) => return Err(e),
567-
};
568-
569555
let maybe_streaming_caps =
570556
if let ClientConnectionResult::ConnectionAccepted(info) = connection_result {
571557
session_manager_lock.update_client_connections(
@@ -593,8 +579,6 @@ fn connection_pipeline(
593579
con_bail!("Only streaming clients are supported for now");
594580
};
595581

596-
dbg_connection!("connection_pipeline: setting up negotiated streaming config");
597-
598582
let initial_settings = session_manager_lock.settings().clone();
599583

600584
fn get_view_res(config: FrameSize, default_res: UVec2) -> UVec2 {
@@ -801,10 +785,6 @@ fn connection_pipeline(
801785
.with_ext(NegotiatedStreamingConfigExt {}),
802786
)
803787
.to_con()?;
804-
proto_socket.send(&stream_config_packet).to_con()?;
805-
806-
let (mut control_sender, mut control_receiver) =
807-
proto_socket.split(STREAMING_RECV_TIMEOUT).to_con()?;
808788

809789
let new_steamvr_hmd_init_config = SteamvrHmdInitConfig {
810790
eye_resolution_width: transcoding_view_resolution.x,
@@ -819,21 +799,38 @@ fn connection_pipeline(
819799
session.steamvr_hmd_init_config = new_steamvr_hmd_init_config;
820800
session.restart_settings_hash = new_hash;
821801

822-
control_sender.send(&ServerControlPacket::Restarting).ok();
802+
alvr_sockets::send_restart_signal(socket, stream_config_packet)?;
823803

824804
crate::notify_restart_driver();
825-
}
826805

827-
dbg_connection!("connection_pipeline: Send StartStream packet");
828-
control_sender
829-
.send(&ServerControlPacket::StartStream)
830-
.to_con()?;
806+
*lifecycle_state.write() = LifecycleState::ShuttingDown;
831807

832-
let signal = control_receiver.recv(HANDSHAKE_ACTION_TIMEOUT)?;
833-
if !matches!(signal, ClientControlPacket::StreamReady) {
834-
con_bail!("Got unexpected packet waiting for stream ack");
808+
return Ok(());
835809
}
836-
dbg_connection!("connection_pipeline: Got StreamReady packet");
810+
811+
let stream_protocol = if wired {
812+
SocketProtocol::Tcp
813+
} else {
814+
initial_settings.connection.stream_protocol
815+
};
816+
817+
dbg_connection!("connection_pipeline: Finishing handshake");
818+
let mut socket = SocketConnection::from_client_connection(
819+
socket,
820+
HANDSHAKE_ACTION_TIMEOUT,
821+
stream_config_packet,
822+
StreamSocketConfig {
823+
protocol: stream_protocol,
824+
port: initial_settings.connection.stream_port,
825+
buffer_config: initial_settings.connection.server_buffer_config,
826+
max_packet_size: initial_settings.connection.packet_size as _,
827+
dscp: initial_settings.connection.dscp,
828+
},
829+
)?;
830+
831+
dbg_connection!("connection_pipeline: Handshake successful, spawning threads");
832+
833+
let disconnect_notif = Arc::new(Condvar::new());
837834

838835
*ctx.statistics_manager.write() = Some(StatisticsManager::new(
839836
initial_settings.connection.statistics_history_size,
@@ -844,36 +841,23 @@ fn connection_pipeline(
844841
0.0
845842
},
846843
));
847-
848844
*ctx.bitrate_manager.lock() =
849845
BitrateManager::new(initial_settings.video.bitrate.history_size, fps);
846+
*ctx.tracking_manager.write() =
847+
TrackingManager::new(initial_settings.connection.statistics_history_size);
850848

851-
let stream_protocol = if wired {
852-
SocketProtocol::Tcp
853-
} else {
854-
initial_settings.connection.stream_protocol
855-
};
856-
857-
dbg_connection!("connection_pipeline: StreamSocket connect_to_client");
858-
let mut stream_socket = StreamSocketBuilder::connect_to_client(
859-
HANDSHAKE_ACTION_TIMEOUT,
860-
client_ip,
861-
initial_settings.connection.stream_port,
862-
stream_protocol,
863-
initial_settings.connection.dscp,
864-
initial_settings.connection.server_buffer_config,
865-
initial_settings.connection.packet_size as _,
866-
)?;
849+
let control_sender = Arc::new(Mutex::new(socket.request_reliable_stream()?));
850+
let mut video_sender = socket.request_unreliable_stream(VIDEO);
851+
let game_audio_sender: alvr_sockets::StreamSender<()> = socket.request_unreliable_stream(AUDIO);
852+
let haptics_sender = socket.request_unreliable_stream(HAPTICS);
867853

868-
let mut video_sender = stream_socket.request_stream(VIDEO);
869-
let game_audio_sender: alvr_sockets::StreamSender<()> = stream_socket.request_stream(AUDIO);
854+
let mut control_receiver = socket.subscribe_to_reliable_stream()?;
870855
let mut microphone_receiver: alvr_sockets::StreamReceiver<()> =
871-
stream_socket.subscribe_to_stream(AUDIO, MAX_UNREAD_PACKETS);
856+
socket.subscribe_to_unreliable_stream(AUDIO, MAX_UNREAD_PACKETS);
872857
let tracking_receiver =
873-
stream_socket.subscribe_to_stream::<TrackingData>(TRACKING, MAX_UNREAD_PACKETS);
874-
let haptics_sender = stream_socket.request_stream(HAPTICS);
858+
socket.subscribe_to_unreliable_stream::<TrackingData>(TRACKING, MAX_UNREAD_PACKETS);
875859
let mut statics_receiver =
876-
stream_socket.subscribe_to_stream::<ClientStatistics>(STATISTICS, MAX_UNREAD_PACKETS);
860+
socket.subscribe_to_unreliable_stream::<ClientStatistics>(STATISTICS, MAX_UNREAD_PACKETS);
877861

878862
let (video_channel_sender, video_channel_receiver) =
879863
std::sync::mpsc::sync_channel(initial_settings.connection.max_queued_server_video_frames);
@@ -906,7 +890,7 @@ fn connection_pipeline(
906890
}
907891
});
908892

909-
#[cfg_attr(target_os = "linux", allow(unused_variables))]
893+
#[cfg_attr(target_os = "linux", expect(unused_variables))]
910894
let game_audio_thread = if let Switch::Enabled(config) =
911895
initial_settings.audio.game_audio.clone()
912896
{
@@ -1059,23 +1043,14 @@ fn connection_pipeline(
10591043
}
10601044
};
10611045

1062-
*ctx.tracking_manager.write() =
1063-
TrackingManager::new(initial_settings.connection.statistics_history_size);
1064-
let hand_gesture_manager = Arc::new(Mutex::new(HandGestureManager::new()));
1065-
10661046
let tracking_receive_thread = thread::spawn({
10671047
let ctx = Arc::clone(&ctx);
1068-
let hand_gesture_manager = Arc::clone(&hand_gesture_manager);
10691048
let initial_settings = initial_settings.clone();
10701049
let client_hostname = client_hostname.clone();
10711050
move || {
1072-
tracking::tracking_loop(
1073-
&ctx,
1074-
initial_settings,
1075-
hand_gesture_manager,
1076-
tracking_receiver,
1077-
|| is_streaming(&client_hostname),
1078-
);
1051+
tracking::tracking_loop(&ctx, initial_settings, tracking_receiver, || {
1052+
is_streaming(&client_hostname)
1053+
});
10791054
}
10801055
});
10811056

@@ -1114,8 +1089,6 @@ fn connection_pipeline(
11141089
}
11151090
});
11161091

1117-
let control_sender = Arc::new(Mutex::new(control_sender));
1118-
11191092
let real_time_update_thread = thread::spawn({
11201093
let control_sender = Arc::clone(&control_sender);
11211094
let client_hostname = client_hostname.clone();
@@ -1341,7 +1314,7 @@ fn connection_pipeline(
13411314
let client_hostname = client_hostname.clone();
13421315
move || {
13431316
while is_streaming(&client_hostname) {
1344-
match stream_socket.recv() {
1317+
match socket.recv_poll() {
13451318
Ok(()) => (),
13461319
Err(ConnectionError::TryAgain(_)) => continue,
13471320
Err(e) => {
@@ -1374,22 +1347,19 @@ fn connection_pipeline(
13741347
}
13751348
});
13761349

1377-
{
1378-
if initial_settings.connection.enable_on_connect_script {
1379-
let on_connect_script = FILESYSTEM_LAYOUT.get().map(|l| l.connect_script()).unwrap();
1380-
info!(
1381-
"Running on connect script (connect): {}",
1382-
on_connect_script.display()
1383-
);
1384-
if let Err(e) = Command::new(&on_connect_script)
1385-
.env("ACTION", "connect")
1386-
.spawn()
1387-
{
1388-
warn!("Failed to run connect script: {e}");
1389-
}
1350+
if initial_settings.connection.enable_on_connect_script {
1351+
let on_connect_script = FILESYSTEM_LAYOUT.get().map(|l| l.connect_script()).unwrap();
1352+
info!(
1353+
"Running on connect script (connect): {}",
1354+
on_connect_script.display()
1355+
);
1356+
if let Err(e) = Command::new(&on_connect_script)
1357+
.env("ACTION", "connect")
1358+
.spawn()
1359+
{
1360+
warn!("Failed to run connect script: {e}");
13901361
}
13911362
}
1392-
13931363
if initial_settings.extra.capture.startup_video_recording {
13941364
info!("Creating recording file");
13951365
crate::create_recording_file(&ctx, session_manager_lock.settings());
@@ -1416,7 +1386,7 @@ fn connection_pipeline(
14161386
))
14171387
.ok();
14181388

1419-
dbg_connection!("connection_pipeline: handshake finished; unlocking streams");
1389+
dbg_connection!("connection_pipeline: Threads initialized; unlocking streams");
14201390
alvr_common::wait_rwlock(&disconnect_notif, &mut session_manager_lock);
14211391
dbg_connection!("connection_pipeline: Begin connection shutdown");
14221392

alvr/server_core/src/lib.rs

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ pub use tracking::HandType;
1616

1717
use crate::connection::VideoPacket;
1818
use alvr_common::{
19-
ConnectionState, DEVICE_ID_TO_PATH, DeviceMotion, LifecycleState, Pose, RelaxedAtomic,
20-
ViewParams, dbg_server_core, error,
19+
ConnectionState, DEVICE_ID_TO_PATH, DeviceMotion, LifecycleState, Pose, ViewParams,
20+
dbg_server_core, error,
2121
glam::{UVec2, Vec2},
2222
parking_lot::{Mutex, RwLock},
2323
settings_schema::Switch,
@@ -185,7 +185,6 @@ pub fn registered_button_set() -> HashSet<u64> {
185185

186186
pub struct ServerCoreContext {
187187
lifecycle_state: Arc<RwLock<LifecycleState>>,
188-
is_restarting: RelaxedAtomic,
189188
connection_context: Arc<ConnectionContext>,
190189
connection_thread: Arc<RwLock<Option<JoinHandle<()>>>>,
191190
webserver_runtime: Option<Runtime>,
@@ -246,9 +245,7 @@ impl ServerCoreContext {
246245
(
247246
Self {
248247
lifecycle_state: Arc::new(RwLock::new(LifecycleState::StartingUp)),
249-
is_restarting: RelaxedAtomic::new(false),
250248
connection_context,
251-
252249
connection_thread: Arc::new(RwLock::new(None)),
253250
webserver_runtime: Some(webserver_runtime),
254251
},
@@ -530,14 +527,6 @@ impl ServerCoreContext {
530527
.as_mut()
531528
.map(|stats| stats.duration_until_next_vsync())
532529
}
533-
534-
pub fn restart(self) {
535-
dbg_server_core!("restart");
536-
537-
self.is_restarting.set(true);
538-
539-
// drop is called here for self
540-
}
541530
}
542531

543532
impl Drop for ServerCoreContext {

0 commit comments

Comments
 (0)