From c494484eb7c0370bde226219e6ecec47c2b747f7 Mon Sep 17 00:00:00 2001 From: Tomer Gilad Date: Wed, 25 Feb 2026 17:25:54 +0200 Subject: [PATCH 1/4] RUST/AGENT: fix agent deadlocks and c-strings Fix a deadlock in fetch_remote_md due to locking of an already locked RwLock. Fix several places calling c++ api with non-null terminated strings. Also make test_etcd_metadata_exchange run on ci and more consistent with the c++ version of the same test. --- .gitlab/test_rust.sh | 20 +++++++++++++++++++- src/bindings/rust/src/agent.rs | 24 ++++++++++++++++-------- src/bindings/rust/tests/tests.rs | 22 ++++++++++++++++++---- 3 files changed, 53 insertions(+), 13 deletions(-) diff --git a/.gitlab/test_rust.sh b/.gitlab/test_rust.sh index 317dc35a5e..e13b2fbf2b 100755 --- a/.gitlab/test_rust.sh +++ b/.gitlab/test_rust.sh @@ -1,5 +1,5 @@ #!/bin/sh -# SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -14,6 +14,10 @@ # See the License for the specific language governing permissions and # limitations under the License. + +# shellcheck disable=SC1091 +. "$(dirname "$0")/../.ci/scripts/common.sh" + set -e set -x @@ -45,6 +49,20 @@ export NIXL_PLUGIN_DIR=${INSTALL_DIR}/lib/$ARCH-linux-gnu/plugins export NIXL_PREFIX=${INSTALL_DIR} export NIXL_NO_STUBS_FALLBACK=1 +echo "==== Running ETCD server ====" +etcd_port=$(get_next_tcp_port) +etcd_peer_port=$(get_next_tcp_port) +export NIXL_ETCD_ENDPOINTS="http://127.0.0.1:${etcd_port}" +export NIXL_ETCD_PEER_URLS="http://127.0.0.1:${etcd_peer_port}" +export NIXL_ETCD_NAMESPACE="/nixl/rust_ci/${etcd_port}" +etcd --listen-client-urls ${NIXL_ETCD_ENDPOINTS} --advertise-client-urls ${NIXL_ETCD_ENDPOINTS} \ + --listen-peer-urls ${NIXL_ETCD_PEER_URLS} --initial-advertise-peer-urls ${NIXL_ETCD_PEER_URLS} \ + --initial-cluster default=${NIXL_ETCD_PEER_URLS} & +ETCD_PID=$! +trap 'kill -9 $ETCD_PID 2>/dev/null || true' EXIT + +wait_for_etcd + cargo test -- --test-threads=1 # test that stubs and real wrapper defined APIs / symbols match diff --git a/src/bindings/rust/src/agent.rs b/src/bindings/rust/src/agent.rs index 68dad55cc0..9ed81dd233 100644 --- a/src/bindings/rust/src/agent.rs +++ b/src/bindings/rust/src/agent.rs @@ -1,4 +1,4 @@ -// SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -697,7 +697,7 @@ impl Agent { tracing::trace!(remote_agent = %remote_name, "Fetching remote metadata from etcd"); let c_remote_name = CString::new(remote_name)?; - let inner_guard = self.inner.write().unwrap(); + let mut inner_guard = self.inner.write().unwrap(); let status = unsafe { bindings::nixl_capi_fetch_remote_md( @@ -709,9 +709,7 @@ impl Agent { match status { NIXL_CAPI_SUCCESS => { - self.inner - .write() - .unwrap() + inner_guard .remotes .insert(remote_name.to_string()); tracing::trace!(remote_agent = %remote_name, "Successfully fetched remote metadata from etcd"); @@ -1111,7 +1109,7 @@ impl AgentInner { fn invalidate_remote_md(&mut self, remote_agent: &str) -> Result<(), NixlError> { unsafe { if self.remotes.remove(remote_agent) { - nixl_capi_invalidate_remote_md(self.handle.as_ptr(), remote_agent.as_ptr().cast()); + nixl_capi_invalidate_remote_md(self.handle.as_ptr(), CString::new(remote_agent)?.as_ptr().cast()); } else { return Err(NixlError::InvalidParam); } @@ -1122,7 +1120,7 @@ impl AgentInner { fn invalidate_all_remotes(&mut self) -> Result<(), NixlError> { unsafe { for remote in self.remotes.drain() { - nixl_capi_invalidate_remote_md(self.handle.as_ptr(), remote.as_ptr().cast()); + nixl_capi_invalidate_remote_md(self.handle.as_ptr(), CString::new(remote.as_str())?.as_ptr().cast()); } } Ok(()) @@ -1136,7 +1134,17 @@ impl Drop for AgentInner { // invalidate all remotes for remote in self.remotes.iter() { tracing::trace!(remote.agent = %remote, "Invalidating remote agent"); - nixl_capi_invalidate_remote_md(self.handle.as_ptr(), remote.as_ptr().cast()); + + let c_remote = match CString::new(remote.as_str()) { + Ok(s) => s, + Err(e) => { + tracing::warn!(remote.agent = %remote, error = ?e, + "Skipping remote invalidation: remote name contains interior NULL"); + continue; + } + }; + + nixl_capi_invalidate_remote_md(self.handle.as_ptr(), c_remote.as_ptr().cast()); } // destroy all backends diff --git a/src/bindings/rust/tests/tests.rs b/src/bindings/rust/tests/tests.rs index c0cac1ba11..45c91a06c3 100644 --- a/src/bindings/rust/tests/tests.rs +++ b/src/bindings/rust/tests/tests.rs @@ -1,4 +1,4 @@ -// SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -20,7 +20,8 @@ //! `nixl` crate. use nixl_sys::*; -use std::env; +use std::time::Duration; +use std::{env, thread}; use std::collections::HashMap; // Helper function to create an agent with error handling fn create_test_agent(name: &str) -> Result { @@ -701,11 +702,16 @@ fn test_etcd_metadata_exchange() -> Result<(), NixlError> { let plugins = agent1.get_available_plugins()?; let plugin_name = find_plugin(&plugins, "UCX")?; let (_mems, params) = agent1.get_plugin_params(&plugin_name)?; - let backend = agent1.create_backend(&plugin_name, ¶ms)?; + let backend1 = agent1.create_backend(&plugin_name, ¶ms)?; + + // Backend on agent2 too + let (_mems2, params2) = agent2.get_plugin_params(&plugin_name)?; + let _backend2 = agent2.create_backend(&plugin_name, ¶ms2)?; + // Create OptArgs with backend let mut opt_args = OptArgs::new()?; - opt_args.add_backend(&backend)?; + opt_args.add_backend(&backend1)?; // Send agent1's metadata to etcd agent1.send_local_md(Some(&opt_args))?; @@ -715,10 +721,18 @@ fn test_etcd_metadata_exchange() -> Result<(), NixlError> { agent2.fetch_remote_md("EtcdAgent1", Some(&opt_args))?; println!("Successfully fetched agent1 metadata from etcd"); + thread::sleep(Duration::from_secs(10)); + // Invalidate agent1's metadata in etcd agent1.invalidate_local_md(Some(&opt_args))?; println!("Successfully invalidated agent1 metadata in etcd"); + thread::sleep(Duration::from_secs(10)); + + agent2.invalidate_remote_md("EtcdAgent1")?; + println!("Successfully invalidated agent2 metadata in etcd"); + thread::sleep(Duration::from_secs(10)); + Ok(()) } From 71451249ea110487d8b6ece06a6192fed4bf5924 Mon Sep 17 00:00:00 2001 From: Tomer Gilad Date: Fri, 27 Feb 2026 12:02:36 +0200 Subject: [PATCH 2/4] RUST/AGENT: fix cr comments Signed-off-by: Tomer Gilad --- .ci/scripts/common.sh | 22 ++++++++++++++++++++ .gitlab/test_cpp.sh | 13 +----------- .gitlab/test_nixlbench.sh | 13 +----------- .gitlab/test_python.sh | 13 +----------- .gitlab/test_rust.sh | 13 +----------- src/bindings/rust/src/agent.rs | 10 +++++++-- src/bindings/rust/tests/tests.rs | 35 +++++++++++++++++++++++++------- 7 files changed, 62 insertions(+), 57 deletions(-) diff --git a/.ci/scripts/common.sh b/.ci/scripts/common.sh index f714f765e8..f0e225795c 100755 --- a/.ci/scripts/common.sh +++ b/.ci/scripts/common.sh @@ -132,3 +132,25 @@ wait_for_etcd() { done echo "Etcd is ready" } + +start_etcd_server() { + local namespace_prefix=$1 + if [ -z "${namespace_prefix}" ]; then + echo "Usage: start_etcd_server " + exit 1 + fi + + echo "==== Running ETCD server ====" + etcd_port=$(get_next_tcp_port) + etcd_peer_port=$(get_next_tcp_port) + export NIXL_ETCD_ENDPOINTS="http://127.0.0.1:${etcd_port}" + export NIXL_ETCD_PEER_URLS="http://127.0.0.1:${etcd_peer_port}" + export NIXL_ETCD_NAMESPACE="${namespace_prefix}/${etcd_port}" + + etcd --listen-client-urls "${NIXL_ETCD_ENDPOINTS}" --advertise-client-urls "${NIXL_ETCD_ENDPOINTS}" \ + --listen-peer-urls "${NIXL_ETCD_PEER_URLS}" --initial-advertise-peer-urls "${NIXL_ETCD_PEER_URLS}" \ + --initial-cluster "default=${NIXL_ETCD_PEER_URLS}" & + ETCD_PID=$! + + wait_for_etcd +} diff --git a/.gitlab/test_cpp.sh b/.gitlab/test_cpp.sh index ae89373003..e0963af88d 100755 --- a/.gitlab/test_cpp.sh +++ b/.gitlab/test_cpp.sh @@ -72,18 +72,7 @@ else echo "/sys/module/nv_peer_mem/version not found" fi -echo "==== Running ETCD server ====" -etcd_port=$(get_next_tcp_port) -etcd_peer_port=$(get_next_tcp_port) -export NIXL_ETCD_ENDPOINTS="http://127.0.0.1:${etcd_port}" -export NIXL_ETCD_PEER_URLS="http://127.0.0.1:${etcd_peer_port}" -export NIXL_ETCD_NAMESPACE="/nixl/cpp_ci/${etcd_port}" -etcd --listen-client-urls ${NIXL_ETCD_ENDPOINTS} --advertise-client-urls ${NIXL_ETCD_ENDPOINTS} \ - --listen-peer-urls ${NIXL_ETCD_PEER_URLS} --initial-advertise-peer-urls ${NIXL_ETCD_PEER_URLS} \ - --initial-cluster default=${NIXL_ETCD_PEER_URLS} & -ETCD_PID=$! - -wait_for_etcd +start_etcd_server "/nixl/cpp_ci" echo "==== Running C++ tests ====" cd ${INSTALL_DIR} diff --git a/.gitlab/test_nixlbench.sh b/.gitlab/test_nixlbench.sh index b8aa778060..d048d613ff 100755 --- a/.gitlab/test_nixlbench.sh +++ b/.gitlab/test_nixlbench.sh @@ -47,18 +47,7 @@ nvidia-smi topo -m || true ibv_devinfo || true uname -a || true -echo "==== Running ETCD server ====" -etcd_port=$(get_next_tcp_port) -etcd_peer_port=$(get_next_tcp_port) -export NIXL_ETCD_ENDPOINTS="http://127.0.0.1:${etcd_port}" -export NIXL_ETCD_PEER_URLS="http://127.0.0.1:${etcd_peer_port}" -export NIXL_ETCD_NAMESPACE="/nixl/nixlbench_ci/${etcd_port}" -etcd --listen-client-urls ${NIXL_ETCD_ENDPOINTS} --advertise-client-urls ${NIXL_ETCD_ENDPOINTS} \ - --listen-peer-urls ${NIXL_ETCD_PEER_URLS} --initial-advertise-peer-urls ${NIXL_ETCD_PEER_URLS} \ - --initial-cluster default=${NIXL_ETCD_PEER_URLS} & -ETCD_PID=$! - -wait_for_etcd +start_etcd_server "/nixl/nixlbench_ci" echo "==== Running Nixlbench tests ====" cd ${INSTALL_DIR} diff --git a/.gitlab/test_python.sh b/.gitlab/test_python.sh index 6d9ab8d4e5..78e732b20b 100755 --- a/.gitlab/test_python.sh +++ b/.gitlab/test_python.sh @@ -68,18 +68,7 @@ $pip3 install --break-system-packages pytest $pip3 install --break-system-packages pytest-timeout $pip3 install --break-system-packages zmq -echo "==== Running ETCD server ====" -etcd_port=$(get_next_tcp_port) -etcd_peer_port=$(get_next_tcp_port) -export NIXL_ETCD_ENDPOINTS="http://127.0.0.1:${etcd_port}" -export NIXL_ETCD_PEER_URLS="http://127.0.0.1:${etcd_peer_port}" -export NIXL_ETCD_NAMESPACE="/nixl/python_ci/${etcd_port}" -etcd --listen-client-urls ${NIXL_ETCD_ENDPOINTS} --advertise-client-urls ${NIXL_ETCD_ENDPOINTS} \ - --listen-peer-urls ${NIXL_ETCD_PEER_URLS} --initial-advertise-peer-urls ${NIXL_ETCD_PEER_URLS} \ - --initial-cluster default=${NIXL_ETCD_PEER_URLS} & -ETCD_PID=$! - -wait_for_etcd +start_etcd_server "/nixl/python_ci" echo "==== Running python tests ====" pytest -s test/python diff --git a/.gitlab/test_rust.sh b/.gitlab/test_rust.sh index e13b2fbf2b..6b59490682 100755 --- a/.gitlab/test_rust.sh +++ b/.gitlab/test_rust.sh @@ -49,20 +49,9 @@ export NIXL_PLUGIN_DIR=${INSTALL_DIR}/lib/$ARCH-linux-gnu/plugins export NIXL_PREFIX=${INSTALL_DIR} export NIXL_NO_STUBS_FALLBACK=1 -echo "==== Running ETCD server ====" -etcd_port=$(get_next_tcp_port) -etcd_peer_port=$(get_next_tcp_port) -export NIXL_ETCD_ENDPOINTS="http://127.0.0.1:${etcd_port}" -export NIXL_ETCD_PEER_URLS="http://127.0.0.1:${etcd_peer_port}" -export NIXL_ETCD_NAMESPACE="/nixl/rust_ci/${etcd_port}" -etcd --listen-client-urls ${NIXL_ETCD_ENDPOINTS} --advertise-client-urls ${NIXL_ETCD_ENDPOINTS} \ - --listen-peer-urls ${NIXL_ETCD_PEER_URLS} --initial-advertise-peer-urls ${NIXL_ETCD_PEER_URLS} \ - --initial-cluster default=${NIXL_ETCD_PEER_URLS} & -ETCD_PID=$! +start_etcd_server "/nixl/rust_ci" trap 'kill -9 $ETCD_PID 2>/dev/null || true' EXIT -wait_for_etcd - cargo test -- --test-threads=1 # test that stubs and real wrapper defined APIs / symbols match diff --git a/src/bindings/rust/src/agent.rs b/src/bindings/rust/src/agent.rs index 9ed81dd233..51b8be8480 100644 --- a/src/bindings/rust/src/agent.rs +++ b/src/bindings/rust/src/agent.rs @@ -1109,7 +1109,10 @@ impl AgentInner { fn invalidate_remote_md(&mut self, remote_agent: &str) -> Result<(), NixlError> { unsafe { if self.remotes.remove(remote_agent) { - nixl_capi_invalidate_remote_md(self.handle.as_ptr(), CString::new(remote_agent)?.as_ptr().cast()); + nixl_capi_invalidate_remote_md( + self.handle.as_ptr(), + CString::new(remote_agent)?.as_ptr().cast(), + ); } else { return Err(NixlError::InvalidParam); } @@ -1120,7 +1123,10 @@ impl AgentInner { fn invalidate_all_remotes(&mut self) -> Result<(), NixlError> { unsafe { for remote in self.remotes.drain() { - nixl_capi_invalidate_remote_md(self.handle.as_ptr(), CString::new(remote.as_str())?.as_ptr().cast()); + nixl_capi_invalidate_remote_md( + self.handle.as_ptr(), + CString::new(remote.as_str())?.as_ptr().cast(), + ); } } Ok(()) diff --git a/src/bindings/rust/tests/tests.rs b/src/bindings/rust/tests/tests.rs index 45c91a06c3..e530233b40 100644 --- a/src/bindings/rust/tests/tests.rs +++ b/src/bindings/rust/tests/tests.rs @@ -20,7 +20,7 @@ //! `nixl` crate. use nixl_sys::*; -use std::time::Duration; +use std::time::{Duration, Instant}; use std::{env, thread}; use std::collections::HashMap; // Helper function to create an agent with error handling @@ -706,32 +706,53 @@ fn test_etcd_metadata_exchange() -> Result<(), NixlError> { // Backend on agent2 too let (_mems2, params2) = agent2.get_plugin_params(&plugin_name)?; - let _backend2 = agent2.create_backend(&plugin_name, ¶ms2)?; + let backend2 = agent2.create_backend(&plugin_name, ¶ms2)?; // Create OptArgs with backend let mut opt_args = OptArgs::new()?; opt_args.add_backend(&backend1)?; + let mut opt_args2 = OptArgs::new()?; + opt_args2.add_backend(&backend2)?; // Send agent1's metadata to etcd agent1.send_local_md(Some(&opt_args))?; println!("Successfully sent agent1 metadata to etcd"); // Fetch agent1's metadata from etcd with agent2 - agent2.fetch_remote_md("EtcdAgent1", Some(&opt_args))?; + agent2.fetch_remote_md("EtcdAgent1", Some(&opt_args2))?; println!("Successfully fetched agent1 metadata from etcd"); - thread::sleep(Duration::from_secs(10)); + let poll_timeout = Duration::from_secs(10); + let poll_interval = Duration::from_millis(100); + let metadata_visible_start = Instant::now(); + loop { + if agent2.check_remote_metadata("EtcdAgent1", None) { + break; + } + if metadata_visible_start.elapsed() >= poll_timeout { + panic!("Timed out waiting for EtcdAgent1 metadata to become visible"); + } + thread::sleep(poll_interval); + } // Invalidate agent1's metadata in etcd agent1.invalidate_local_md(Some(&opt_args))?; println!("Successfully invalidated agent1 metadata in etcd"); - thread::sleep(Duration::from_secs(10)); + let local_invalidate_start = Instant::now(); + loop { + if !agent2.check_remote_metadata("EtcdAgent1", None) { + break; + } + if local_invalidate_start.elapsed() >= poll_timeout { + panic!("Timed out waiting for EtcdAgent1 metadata invalidation in etcd"); + } + thread::sleep(poll_interval); + } agent2.invalidate_remote_md("EtcdAgent1")?; - println!("Successfully invalidated agent2 metadata in etcd"); - thread::sleep(Duration::from_secs(10)); + println!("Successfully invalidated agent2 cached remote metadata"); Ok(()) } From def487e46fb567baf90a189c4c6117a7fc8ec74e Mon Sep 17 00:00:00 2001 From: Tomer Gilad Date: Fri, 27 Feb 2026 14:45:50 +0200 Subject: [PATCH 3/4] CI: update image tag Signed-off-by: Tomer Gilad --- .ci/jenkins/lib/build-matrix.yaml | 2 +- .ci/jenkins/lib/test-matrix.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.ci/jenkins/lib/build-matrix.yaml b/.ci/jenkins/lib/build-matrix.yaml index 0177fbb7db..1d2e3ca12c 100644 --- a/.ci/jenkins/lib/build-matrix.yaml +++ b/.ci/jenkins/lib/build-matrix.yaml @@ -43,7 +43,7 @@ env: TEST_TIMEOUT: 30 UCX_TLS: "^shm" STORAGE_DRIVER: 'overlay' - CI_IMAGE_TAG: "20260219-1" + CI_IMAGE_TAG: "20260227-1" runs_on_dockers: diff --git a/.ci/jenkins/lib/test-matrix.yaml b/.ci/jenkins/lib/test-matrix.yaml index 1fe42a0bdc..936d34bd07 100644 --- a/.ci/jenkins/lib/test-matrix.yaml +++ b/.ci/jenkins/lib/test-matrix.yaml @@ -49,7 +49,7 @@ env: SLURM_JOB_TIMEOUT: '02:20:00' SLURM_IMMEDIATE_TIMEOUT: "3600" STORAGE_DRIVER: overlay - CI_IMAGE_TAG: "20260219-1" + CI_IMAGE_TAG: "20260227-1" empty_volumes: - {mountPath: /var/lib/containers/storage, memory: false} From 9d6e9b6667893d73b8575a61b67c2bedaa6e6af9 Mon Sep 17 00:00:00 2001 From: Tomer Gilad Date: Mon, 2 Mar 2026 12:44:06 +0200 Subject: [PATCH 4/4] CI: Update CI_IMAGE_TAG after merge --- .ci/jenkins/lib/build-matrix.yaml | 2 +- .ci/jenkins/lib/test-matrix.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.ci/jenkins/lib/build-matrix.yaml b/.ci/jenkins/lib/build-matrix.yaml index 6b5585fbf9..f4c7985c87 100644 --- a/.ci/jenkins/lib/build-matrix.yaml +++ b/.ci/jenkins/lib/build-matrix.yaml @@ -43,7 +43,7 @@ env: TEST_TIMEOUT: 30 UCX_TLS: "^shm" STORAGE_DRIVER: 'overlay' - CI_IMAGE_TAG: "20260226-1" + CI_IMAGE_TAG: "20260302-1" runs_on_dockers: diff --git a/.ci/jenkins/lib/test-matrix.yaml b/.ci/jenkins/lib/test-matrix.yaml index 5f627c111f..f562e8487a 100644 --- a/.ci/jenkins/lib/test-matrix.yaml +++ b/.ci/jenkins/lib/test-matrix.yaml @@ -49,7 +49,7 @@ env: SLURM_JOB_TIMEOUT: '02:20:00' SLURM_IMMEDIATE_TIMEOUT: "3600" STORAGE_DRIVER: overlay - CI_IMAGE_TAG: "20260226-1" + CI_IMAGE_TAG: "20260302-1" empty_volumes: - {mountPath: /var/lib/containers/storage, memory: false}