Skip to content

Commit 8022cc0

Browse files
committed
Add concurrency CI failure checks
Signed-off-by: lucarlig <luca.carlig@ibm.com>
1 parent 2c4c1be commit 8022cc0

13 files changed

Lines changed: 392 additions & 10 deletions

File tree

.github/workflows/ci.yml

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,25 @@ jobs:
5555
tool: cargo-nextest
5656
- run: cargo nextest run --locked --workspace
5757

58+
loom:
59+
runs-on: ubuntu-latest
60+
steps:
61+
- uses: actions/checkout@v6.0.2
62+
- uses: dtolnay/rust-toolchain@stable
63+
- uses: Swatinem/rust-cache@v2.9.1
64+
- run: cargo test --locked -p contextforge-gateway-rs-lib --features loom-tests,internal-test-hooks --test concurrency_loom
65+
66+
miri:
67+
runs-on: ubuntu-latest
68+
steps:
69+
- uses: actions/checkout@v6.0.2
70+
- uses: dtolnay/rust-toolchain@nightly
71+
with:
72+
components: miri
73+
- uses: Swatinem/rust-cache@v2.9.1
74+
- run: cargo miri setup
75+
- run: cargo miri test --locked -p contextforge-gateway-rs-lib --features miri-tests,internal-test-hooks --test miri_checks
76+
5877
build:
5978
runs-on: ubuntu-latest
6079
steps:

Cargo.lock

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

crates/contextforge-gateway-rs-lib/Cargo.toml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,9 @@ typed-builder.workspace = true
5555
[features]
5656
default = []
5757
with_tools = []
58+
loom-tests = []
59+
miri-tests = []
60+
internal-test-hooks = []
5861

5962

6063
[dev-dependencies]
@@ -64,6 +67,7 @@ axum-test = "20.0.0"
6467
test-log = "0.2.20"
6568
axum-server = { version = "0.8.0", features = ["tls-rustls"] }
6669
futures.workspace = true
70+
loom = "0.7.2"
6771

6872
[lints]
6973
workspace = true

crates/contextforge-gateway-rs-lib/src/gateway/mcp_gateway.rs

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ use crate::{
2929
SessionId,
3030
gateway::{
3131
mcp_call_validator::InitializeCallValidator,
32-
session_manager::SessionManager,
32+
session_manager::{SessionManager, return_transport_entry},
3333
session_store::{UserSession, UserSessionStore},
3434
},
3535
};
@@ -252,7 +252,8 @@ where
252252

253253
let mut transports = self.transports.lock().await;
254254
for (name, svc) in backend_services {
255-
transports.entry(BackendTransportKey::from((&name, session_id))).and_modify(|e| e.service = svc);
255+
let key = BackendTransportKey::from((&name, session_id));
256+
return_transport_entry(&mut transports, &key, svc, |entry| &mut entry.service);
256257
}
257258
drop(transports);
258259

@@ -399,7 +400,8 @@ where
399400

400401
let mut transports = self.transports.lock().await;
401402
for (name, svc) in backend_services {
402-
transports.entry(BackendTransportKey::from((&name, session_id))).and_modify(|e| e.service = svc);
403+
let key = BackendTransportKey::from((&name, session_id));
404+
return_transport_entry(&mut transports, &key, svc, |entry| &mut entry.service);
403405
}
404406
drop(transports);
405407

@@ -781,6 +783,17 @@ fn split_resource_name<'a, T: AsRef<str>, N: AsRef<str>>(
781783
None
782784
}
783785

786+
#[cfg(feature = "internal-test-hooks")]
787+
pub fn test_split_tool_name_owned(tool_name: &str, backend_names: &[&str]) -> Option<(String, String)> {
788+
split_tool_name(&tool_name, backend_names).map(|pair| (pair.backend_name.to_owned(), pair.tool_name.to_owned()))
789+
}
790+
791+
#[cfg(feature = "internal-test-hooks")]
792+
pub fn test_split_resource_name_owned(resource_uri: &str, backend_names: &[&str]) -> Option<(String, String)> {
793+
split_resource_name(&resource_uri, backend_names)
794+
.map(|pair| (pair.backend_name.to_owned(), pair.resource_uri.to_owned()))
795+
}
796+
784797
#[cfg(test)]
785798
mod tests {
786799
// Note this useful idiom: importing names from outer (for mod tests) scope.

crates/contextforge-gateway-rs-lib/src/gateway/mod.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,13 @@ mod session_manager;
44
mod session_store;
55

66
pub use mcp_gateway::{LocalUserSessionStore, McpService};
7+
8+
#[cfg(feature = "internal-test-hooks")]
9+
#[doc(hidden)]
10+
pub mod test_support {
11+
pub use super::mcp_gateway::{test_split_resource_name_owned, test_split_tool_name_owned};
12+
pub use super::session_manager::{borrow_transport_entry, return_transport_entry};
13+
pub use super::session_store::test_support::{
14+
test_session_mapping_msgpack_round_trip, test_user_session_msgpack_round_trip,
15+
};
16+
}

crates/contextforge-gateway-rs-lib/src/gateway/session_manager.rs

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1-
use std::{collections::HashMap, sync::Arc};
1+
use std::{
2+
collections::HashMap,
3+
hash::{BuildHasher, Hash},
4+
sync::Arc,
5+
};
26

37
use contextforge_gateway_rs_apis::user_store::VirtualHost;
48
use tokio::sync::Mutex;
@@ -7,6 +11,32 @@ use tracing::{debug, info};
711
use super::mcp_gateway::{BackendTransportKey, BackendTransportService, ServiceHolder};
812
use crate::layers::session_id::SessionId;
913

14+
pub fn borrow_transport_entry<K, V, Service, Hasher>(
15+
transports: &mut HashMap<K, V, Hasher>,
16+
key: &K,
17+
service_slot: impl FnOnce(&mut V) -> &mut Option<Service>,
18+
) -> Option<Option<Service>>
19+
where
20+
K: Eq + Hash,
21+
Hasher: BuildHasher,
22+
{
23+
transports.get_mut(key).map(|entry| service_slot(entry).take())
24+
}
25+
26+
pub fn return_transport_entry<K, V, Service, Hasher>(
27+
transports: &mut HashMap<K, V, Hasher>,
28+
key: &K,
29+
running_service: Option<Service>,
30+
service_slot: impl FnOnce(&mut V) -> &mut Option<Service>,
31+
) where
32+
K: Eq + Hash,
33+
Hasher: BuildHasher,
34+
{
35+
if let Some(entry) = transports.get_mut(key) {
36+
*service_slot(entry) = running_service;
37+
}
38+
}
39+
1040
pub struct SessionManager<'a> {
1141
virtual_host: &'a VirtualHost,
1242
session_id: &'a SessionId,
@@ -32,9 +62,9 @@ impl<'a> SessionManager<'a> {
3262
names
3363
.into_iter()
3464
.filter_map(|name| {
35-
transports
36-
.get_mut(&BackendTransportKey::from((&name, self.session_id)))
37-
.map(|b| ServiceHolder::new(name, b.service.take()))
65+
let key = BackendTransportKey::from((&name, self.session_id));
66+
borrow_transport_entry(&mut transports, &key, |entry| &mut entry.service)
67+
.map(|service| ServiceHolder::new(name, service))
3868
})
3969
.collect()
4070
}
@@ -44,9 +74,8 @@ impl<'a> SessionManager<'a> {
4474
info!("Returning transports {:?} {backend_transports:?}", self.session_id);
4575
let mut transports = self.transports.lock().await;
4676
for svc_holder in backend_transports {
47-
transports
48-
.entry(BackendTransportKey::from((&svc_holder.name, self.session_id)))
49-
.and_modify(|e| e.service = svc_holder.running_service);
77+
let key = BackendTransportKey::from((&svc_holder.name, self.session_id));
78+
return_transport_entry(&mut transports, &key, svc_holder.running_service, |entry| &mut entry.service);
5079
}
5180
}
5281

crates/contextforge-gateway-rs-lib/src/gateway/session_store/mod.rs

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,54 @@ impl UserSession {
6969
}
7070
}
7171

72+
#[cfg(feature = "internal-test-hooks")]
73+
pub(super) mod test_support {
74+
use std::sync::Arc;
75+
76+
use serde::Deserialize;
77+
78+
use super::{SessionMapping, UserSession};
79+
80+
#[derive(Debug, Deserialize, PartialEq, Eq)]
81+
struct OwnedUserSession {
82+
name: String,
83+
principal: String,
84+
downstream_session_id: Arc<str>,
85+
}
86+
87+
pub fn test_session_mapping_msgpack_round_trip(entries: &[(&str, Option<&str>)]) -> Vec<(String, Option<String>)> {
88+
let mut mapping = SessionMapping::new();
89+
for (backend_name, upstream_session_id) in entries {
90+
let upstream_session_id = upstream_session_id.map(Arc::<str>::from);
91+
mapping.push((*backend_name).to_owned(), upstream_session_id.as_ref());
92+
}
93+
94+
let encoded = rmp_serde::encode::to_vec(&mapping).expect("session mapping should encode");
95+
let decoded: SessionMapping = rmp_serde::decode::from_slice(&encoded).expect("session mapping should decode");
96+
97+
entries
98+
.iter()
99+
.map(|(backend_name, _)| {
100+
let upstream_session_id =
101+
decoded.get(backend_name).and_then(super::SessionMap::session).map(|id| id.to_string());
102+
((*backend_name).to_owned(), upstream_session_id)
103+
})
104+
.collect()
105+
}
106+
107+
pub fn test_user_session_msgpack_round_trip(principal: &str, downstream_session_id: &str) -> bool {
108+
let session = UserSession::new(principal.to_owned(), Arc::<str>::from(downstream_session_id));
109+
let encoded = rmp_serde::encode::to_vec(&session).expect("user session should encode");
110+
let decoded: OwnedUserSession = rmp_serde::decode::from_slice(&encoded).expect("user session should decode");
111+
decoded
112+
== OwnedUserSession {
113+
name: "UserSession".to_owned(),
114+
principal: principal.to_owned(),
115+
downstream_session_id: Arc::<str>::from(downstream_session_id),
116+
}
117+
}
118+
}
119+
72120
#[async_trait]
73121
pub trait UserSessionStore: Send + Sync {
74122
async fn get_session<'a>(&self, key: &'a UserSession) -> Result<Option<SessionMapping>, SessionStoreError>;

crates/contextforge-gateway-rs-lib/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@ use transports::{DownstreamTls, Tcp};
2929
use typed_builder::TypedBuilder;
3030

3131
pub use crate::common::{Config, LogRotation};
32+
#[cfg(feature = "internal-test-hooks")]
33+
#[doc(hidden)]
34+
pub use crate::gateway::test_support;
3235

3336
pub type Error = Box<dyn std::error::Error + Send + Sync + 'static>;
3437
pub type Result<T> = std::result::Result<T, Error>;

0 commit comments

Comments
 (0)