forked from 1b-agent/opencadstudio-upstream
feat(plugin): harden V4 host integration
Preserve legacy wire indices and route concurrent V4 responses and background requests by correlation and stable tab ID.
This commit is contained in:
commit
f247d15f6e
110 files changed
+32159
-1254
No files matched your search
@@ -143,6 +143,25 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn v4_additions_preserve_legacy_discriminants() {
|
||||
fn discriminant<T: serde::Serialize>(value: &T) -> u32 {
|
||||
let bytes = bincode::serialize(value).unwrap();
|
||||
u32::from_le_bytes(bytes[..4].try_into().unwrap())
|
||||
}
|
||||
|
||||
assert_eq!(discriminant(&HostRequest::Shutdown), 6);
|
||||
assert_eq!(discriminant(&HostResponse::Error(String::new())), 5);
|
||||
assert_eq!(
|
||||
discriminant(&crate::ipc::protocol::PluginRequest::BumpGeometry),
|
||||
6
|
||||
);
|
||||
assert_eq!(
|
||||
discriminant(&crate::ipc::protocol::PluginResponse::Record(None)),
|
||||
3
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn transport_rejects_oversized_message() {
|
||||
let (mut a, _b) = connect_pair();
|
||||
|
||||
@@ -59,13 +59,13 @@ pub enum HostRequest {
|
||||
NeedsEntityPick {
|
||||
command_id: u64,
|
||||
},
|
||||
Shutdown,
|
||||
ExecuteCode {
|
||||
command_id: u64,
|
||||
source: CommandSource,
|
||||
code: String,
|
||||
tab_index: usize,
|
||||
},
|
||||
Shutdown,
|
||||
}
|
||||
|
||||
/// Responses the plugin runner sends back for `HostRequest`.
|
||||
@@ -76,8 +76,8 @@ pub enum HostResponse {
|
||||
Text(String),
|
||||
Ribbon(Vec<OwnedRibbonGroup>),
|
||||
Manifest(OwnedPluginManifest),
|
||||
CodeExecutionResult(crate::host::ExecutionResult),
|
||||
Error(String),
|
||||
CodeExecutionResult(crate::host::ExecutionResult),
|
||||
}
|
||||
|
||||
/// Requests the plugin runner sends to the host.
|
||||
@@ -87,8 +87,6 @@ pub enum PluginRequest {
|
||||
PushOutput(String),
|
||||
PushError(String),
|
||||
AddEntity(EntityType),
|
||||
/// Add multiple entities in a single request.
|
||||
AddEntities(Vec<EntityType>),
|
||||
/// Replace the existing entity carrying this entity's handle in place.
|
||||
UpdateEntity(EntityType),
|
||||
/// Delete the entity with `handle`.
|
||||
@@ -119,6 +117,8 @@ pub enum PluginRequest {
|
||||
/// Ask the host to create/refresh a shared-memory document view and return
|
||||
/// the file path + current version.
|
||||
OpenDocumentView,
|
||||
/// Add multiple entities in a single request.
|
||||
AddEntities(Vec<EntityType>),
|
||||
/// V4: ask the host to create/refresh a tab-keyed shared-memory document
|
||||
/// view and return the file path + current version.
|
||||
OpenDocumentViewV4 { tab_id: u64 },
|
||||
@@ -134,7 +134,6 @@ pub enum PluginResponse {
|
||||
Ok,
|
||||
Bool(bool),
|
||||
Handle(Handle),
|
||||
Handles(Vec<Handle>),
|
||||
Record(Option<ExtendedDataRecord>),
|
||||
Document(Box<CadDocument>),
|
||||
Error(String),
|
||||
@@ -143,6 +142,7 @@ pub enum PluginResponse {
|
||||
path: String,
|
||||
version: u64,
|
||||
},
|
||||
Handles(Vec<Handle>),
|
||||
/// V4: path to the tab-keyed memory-mapped file and current version.
|
||||
DocumentViewV4 {
|
||||
path: String,
|
||||
|
||||
@@ -11,7 +11,7 @@
|
||||
//! formats or enum variants.
|
||||
|
||||
use std::io::{Read, Write};
|
||||
use std::net::{TcpListener, TcpStream};
|
||||
use std::net::{Shutdown, TcpListener, TcpStream};
|
||||
use std::sync::Arc;
|
||||
use std::sync::Mutex;
|
||||
use std::time::Duration;
|
||||
@@ -82,7 +82,7 @@ pub fn recv_framed<R: Read, T: serde::de::DeserializeOwned>(reader: &mut R) -> R
|
||||
/// Thread-safe [`PluginRequestSender`] implementation that forwards requests
|
||||
/// over a single TCP stream to a request proxy server.
|
||||
pub struct ProxyPluginRequestSender {
|
||||
stream: Mutex<TcpStream>,
|
||||
stream: Arc<Mutex<TcpStream>>,
|
||||
}
|
||||
|
||||
impl ProxyPluginRequestSender {
|
||||
@@ -98,21 +98,14 @@ impl ProxyPluginRequestSender {
|
||||
stream.flush()?;
|
||||
stream.set_read_timeout(Some(REQUEST_TIMEOUT))?;
|
||||
Ok(Self {
|
||||
stream: Mutex::new(stream),
|
||||
stream: Arc::new(Mutex::new(stream)),
|
||||
})
|
||||
}
|
||||
|
||||
/// Create a second handle to the same underlying socket so multiple
|
||||
/// callers can share the proxy connection (requests are still serialized
|
||||
/// by the mutex on each handle).
|
||||
/// Create a second handle that shares the connection and its request lock.
|
||||
pub fn try_clone(&self) -> Result<Self, ProxyError> {
|
||||
let stream = self
|
||||
.stream
|
||||
.lock()
|
||||
.map_err(|e| ProxyError::Io(std::io::Error::other(e.to_string())))?
|
||||
.try_clone()?;
|
||||
Ok(Self {
|
||||
stream: Mutex::new(stream),
|
||||
stream: Arc::clone(&self.stream),
|
||||
})
|
||||
}
|
||||
|
||||
@@ -145,6 +138,7 @@ impl ProxyPluginRequestSender {
|
||||
|| e.kind() == ErrorKind::TimedOut =>
|
||||
{
|
||||
if let Err(e) = poll() {
|
||||
let _ = stream.shutdown(Shutdown::Both);
|
||||
break Err(e);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -43,7 +43,7 @@ type NotificationQueue = mpsc::Receiver<(Option<u64>, HostNotification)>;
|
||||
struct Shared {
|
||||
writer: Mutex<Stream>,
|
||||
next_id: AtomicU64,
|
||||
in_flight: Mutex<Option<mpsc::Sender<(u64, PluginResponse)>>>,
|
||||
in_flight: Mutex<HashMap<u64, mpsc::Sender<PluginResponse>>>,
|
||||
}
|
||||
|
||||
/// A frame the V4 reader thread delivers to the plugin runner loop.
|
||||
@@ -96,7 +96,7 @@ impl V4Client {
|
||||
let shared = Arc::new(Shared {
|
||||
writer: Mutex::new(stream),
|
||||
next_id: AtomicU64::new(1),
|
||||
in_flight: Mutex::new(None),
|
||||
in_flight: Mutex::new(HashMap::new()),
|
||||
});
|
||||
|
||||
let shared_for_reader = Arc::clone(&shared);
|
||||
@@ -122,6 +122,15 @@ impl V4Client {
|
||||
self.runner_queue.recv()
|
||||
}
|
||||
|
||||
/// Wait up to `timeout` for a host request so the runner can also drain
|
||||
/// notifications while otherwise idle.
|
||||
pub fn recv_runner_frame_timeout(
|
||||
&self,
|
||||
timeout: Duration,
|
||||
) -> Result<RunnerFrame, mpsc::RecvTimeoutError> {
|
||||
self.runner_queue.recv_timeout(timeout)
|
||||
}
|
||||
|
||||
/// Non-blocking poll for host-to-plugin notifications.
|
||||
pub fn try_recv_notification(&self) -> Option<(Option<u64>, HostNotification)> {
|
||||
self.notifications.lock().unwrap_or_else(|e| e.into_inner()).try_recv().ok()
|
||||
@@ -214,8 +223,13 @@ fn reader_thread(
|
||||
loop {
|
||||
match recv::<HostToPluginV4>(&mut reader) {
|
||||
Ok(HostToPluginV4::Response { id, payload }) => {
|
||||
if let Some(tx) = shared.in_flight.lock().unwrap_or_else(|e| e.into_inner()).take() {
|
||||
let _ = tx.send((id, payload));
|
||||
let tx = shared
|
||||
.in_flight
|
||||
.lock()
|
||||
.unwrap_or_else(|e| e.into_inner())
|
||||
.remove(&id);
|
||||
if let Some(tx) = tx {
|
||||
let _ = tx.send(payload);
|
||||
} else {
|
||||
eprintln!("[plugin] V4 reader: unexpected response id={id}");
|
||||
}
|
||||
@@ -249,8 +263,11 @@ fn reader_thread(
|
||||
}
|
||||
}
|
||||
}));
|
||||
// Ensure any synchronous waiter is unblocked.
|
||||
let _ = shared.in_flight.lock().unwrap_or_else(|e| e.into_inner()).take();
|
||||
shared
|
||||
.in_flight
|
||||
.lock()
|
||||
.unwrap_or_else(|e| e.into_inner())
|
||||
.clear();
|
||||
}
|
||||
|
||||
/// `HostApi` implementation used inside the plugin process over the V4
|
||||
@@ -264,7 +281,7 @@ pub struct V4PluginHostApi {
|
||||
next_command_id: Cell<u64>,
|
||||
record_cache: RefCell<HashMap<(Handle, String), &'static ExtendedDataRecord>>,
|
||||
doc_view: RefCell<Option<DocumentViewInfo>>,
|
||||
doc_view_v4: RefCell<Option<DocumentViewInfo>>,
|
||||
doc_view_v4: RefCell<Option<(u64, DocumentViewInfo)>>,
|
||||
tab_id_cache: Cell<Option<u64>>,
|
||||
}
|
||||
|
||||
@@ -293,7 +310,7 @@ impl V4PluginHostApi {
|
||||
&self,
|
||||
req: PluginRequest,
|
||||
) -> Result<PluginResponse, crate::ipc::transport::TransportError> {
|
||||
send_plugin_request(&self.shared, req)
|
||||
send_plugin_request(&self.shared, self.tab_id_cache.get(), req)
|
||||
}
|
||||
|
||||
fn fetch_document(&self) -> CadDocument {
|
||||
@@ -316,23 +333,37 @@ impl V4PluginHostApi {
|
||||
/// thread-safe [`V4PluginRequestSender`].
|
||||
fn send_plugin_request(
|
||||
shared: &Arc<Shared>,
|
||||
tab_id: Option<u64>,
|
||||
req: PluginRequest,
|
||||
) -> Result<PluginResponse, crate::ipc::transport::TransportError> {
|
||||
let id = shared.next_id.fetch_add(1, Ordering::Relaxed);
|
||||
let (tx, rx) = mpsc::channel();
|
||||
*shared.in_flight.lock().unwrap_or_else(|e| e.into_inner()) = Some(tx);
|
||||
{
|
||||
shared
|
||||
.in_flight
|
||||
.lock()
|
||||
.unwrap_or_else(|e| e.into_inner())
|
||||
.insert(id, tx);
|
||||
let send_result = {
|
||||
let mut writer = shared.writer.lock().unwrap_or_else(|e| e.into_inner());
|
||||
send(&mut writer, &PluginToHostV4::Request { id, payload: req })?;
|
||||
send(
|
||||
&mut writer,
|
||||
&PluginToHostV4::Request {
|
||||
id,
|
||||
tab_id,
|
||||
payload: req,
|
||||
},
|
||||
)
|
||||
};
|
||||
if let Err(error) = send_result {
|
||||
shared
|
||||
.in_flight
|
||||
.lock()
|
||||
.unwrap_or_else(|e| e.into_inner())
|
||||
.remove(&id);
|
||||
return Err(error);
|
||||
}
|
||||
match rx.recv() {
|
||||
Ok((resp_id, resp)) if resp_id == id => Ok(resp),
|
||||
Ok((resp_id, _)) => Err(crate::ipc::transport::TransportError::Io(
|
||||
std::io::Error::new(
|
||||
std::io::ErrorKind::InvalidData,
|
||||
format!("V4 response id mismatch: expected {id}, got {resp_id}"),
|
||||
),
|
||||
)),
|
||||
Ok(resp) => Ok(resp),
|
||||
Err(_) => Err(crate::ipc::transport::TransportError::Io(
|
||||
std::io::Error::new(
|
||||
std::io::ErrorKind::NotConnected,
|
||||
@@ -343,17 +374,16 @@ fn send_plugin_request(
|
||||
}
|
||||
|
||||
/// Thread-safe wrapper around the V4 connection so worker threads can issue
|
||||
/// host requests. A per-sender mutex serializes requests, matching the
|
||||
/// single-in-flight behavior of [`V4PluginHostApi::request`].
|
||||
/// host requests for the originating document tab.
|
||||
struct V4PluginRequestSender {
|
||||
shared: Arc<Shared>,
|
||||
lock: Mutex<()>,
|
||||
tab_id: u64,
|
||||
}
|
||||
|
||||
impl PluginRequestSender for V4PluginRequestSender {
|
||||
fn request(&self, req: PluginRequest) -> Result<PluginResponse, PluginRequestError> {
|
||||
let _guard = self.lock.lock().map_err(|e| PluginRequestError(e.to_string()))?;
|
||||
send_plugin_request(&self.shared, req).map_err(|e| PluginRequestError(e.to_string()))
|
||||
send_plugin_request(&self.shared, Some(self.tab_id), req)
|
||||
.map_err(|e| PluginRequestError(e.to_string()))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -631,7 +661,7 @@ impl HostApi for V4PluginHostApi {
|
||||
fn plugin_request_sender(&self) -> Option<Box<dyn PluginRequestSender>> {
|
||||
Some(Box::new(V4PluginRequestSender {
|
||||
shared: Arc::clone(&self.shared),
|
||||
lock: Mutex::new(()),
|
||||
tab_id: self.tab_id(),
|
||||
}))
|
||||
}
|
||||
|
||||
@@ -655,14 +685,13 @@ impl HostApi for V4PluginHostApi {
|
||||
}
|
||||
}
|
||||
|
||||
fn document_view_v4(&mut self, _tab_id: u64) -> Option<DocumentViewInfo> {
|
||||
fn document_view_v4(&mut self, tab_id: u64) -> Option<DocumentViewInfo> {
|
||||
{
|
||||
let mut view = self.doc_view_v4.borrow_mut();
|
||||
if view.is_none() {
|
||||
let tab_id = self.tab_id();
|
||||
if view.as_ref().map(|(cached_id, _)| *cached_id) != Some(tab_id) {
|
||||
match self.request(PluginRequest::OpenDocumentViewV4 { tab_id }) {
|
||||
Ok(PluginResponse::DocumentViewV4 { path, version }) => {
|
||||
*view = Some(DocumentViewInfo { path, version });
|
||||
*view = Some((tab_id, DocumentViewInfo { path, version }));
|
||||
}
|
||||
Ok(other) => {
|
||||
eprintln!("[plugin] unexpected OpenDocumentViewV4 response: {other:?}");
|
||||
@@ -673,13 +702,18 @@ impl HostApi for V4PluginHostApi {
|
||||
}
|
||||
}
|
||||
}
|
||||
self.doc_view_v4.borrow().clone()
|
||||
self.doc_view_v4
|
||||
.borrow()
|
||||
.as_ref()
|
||||
.map(|(_, info)| info.clone())
|
||||
}
|
||||
|
||||
fn close_document_view_v4(&mut self, _tab_id: u64) {
|
||||
let tab_id = self.tab_id();
|
||||
fn close_document_view_v4(&mut self, tab_id: u64) {
|
||||
let _ = self.request(PluginRequest::CloseDocumentViewV4 { tab_id });
|
||||
self.doc_view_v4.borrow_mut().take();
|
||||
let mut view = self.doc_view_v4.borrow_mut();
|
||||
if view.as_ref().map(|(cached_id, _)| *cached_id) == Some(tab_id) {
|
||||
view.take();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -694,7 +728,7 @@ impl V4Client {
|
||||
let shared = Arc::new(Shared {
|
||||
writer: Mutex::new(stream),
|
||||
next_id: AtomicU64::new(1),
|
||||
in_flight: Mutex::new(None),
|
||||
in_flight: Mutex::new(HashMap::new()),
|
||||
});
|
||||
let shared_for_reader = Arc::clone(&shared);
|
||||
let notifications = Arc::new(Mutex::new(notify_rx));
|
||||
@@ -776,7 +810,11 @@ mod tests {
|
||||
let runner = thread::spawn(move || {
|
||||
let req = recv::<PluginToHostV4>(&mut runner_stream).unwrap();
|
||||
match req {
|
||||
PluginToHostV4::Request { id, payload: PluginRequest::PushInfo(s) } => {
|
||||
PluginToHostV4::Request {
|
||||
id,
|
||||
tab_id: _,
|
||||
payload: PluginRequest::PushInfo(s),
|
||||
} => {
|
||||
assert_eq!(s, "hello host");
|
||||
send(
|
||||
&mut runner_stream,
|
||||
@@ -804,7 +842,11 @@ mod tests {
|
||||
let runner = thread::spawn(move || {
|
||||
let req = recv::<PluginToHostV4>(&mut runner_stream).unwrap();
|
||||
match req {
|
||||
PluginToHostV4::Request { id, payload: PluginRequest::AddEntities(v) } => {
|
||||
PluginToHostV4::Request {
|
||||
id,
|
||||
tab_id: _,
|
||||
payload: PluginRequest::AddEntities(v),
|
||||
} => {
|
||||
assert_eq!(v.len(), 2);
|
||||
send(
|
||||
&mut runner_stream,
|
||||
@@ -828,6 +870,56 @@ mod tests {
|
||||
runner.join().unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn concurrent_requests_match_out_of_order_responses() {
|
||||
let (host_stream, mut runner_stream) = connect_pair();
|
||||
let client = V4Client::from_stream_for_test(host_stream);
|
||||
let sender = Arc::new(V4PluginRequestSender {
|
||||
shared: Arc::clone(&client.shared),
|
||||
tab_id: 42,
|
||||
});
|
||||
|
||||
let runner = thread::spawn(move || {
|
||||
let mut requests = Vec::new();
|
||||
for _ in 0..2 {
|
||||
match recv::<PluginToHostV4>(&mut runner_stream).unwrap() {
|
||||
PluginToHostV4::Request {
|
||||
id,
|
||||
tab_id: Some(42),
|
||||
payload: PluginRequest::PushInfo(message),
|
||||
} => requests.push((id, message)),
|
||||
other => panic!("unexpected: {other:?}"),
|
||||
}
|
||||
}
|
||||
for (id, message) in requests.into_iter().rev() {
|
||||
send(
|
||||
&mut runner_stream,
|
||||
&HostToPluginV4::Response {
|
||||
id,
|
||||
payload: PluginResponse::Bool(message == "first"),
|
||||
},
|
||||
)
|
||||
.unwrap();
|
||||
}
|
||||
});
|
||||
|
||||
let first_sender = Arc::clone(&sender);
|
||||
let first = thread::spawn(move || {
|
||||
first_sender
|
||||
.request(PluginRequest::PushInfo("first".to_string()))
|
||||
.unwrap()
|
||||
});
|
||||
let second = thread::spawn(move || {
|
||||
sender
|
||||
.request(PluginRequest::PushInfo("second".to_string()))
|
||||
.unwrap()
|
||||
});
|
||||
|
||||
assert!(matches!(first.join().unwrap(), PluginResponse::Bool(true)));
|
||||
assert!(matches!(second.join().unwrap(), PluginResponse::Bool(false)));
|
||||
runner.join().unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn v4_client_notification_queue_size_env_is_honored() {
|
||||
let _guard = ENV_LOCK.lock().unwrap();
|
||||
|
||||
@@ -29,7 +29,11 @@ pub enum HostToPluginV4 {
|
||||
#[derive(Debug, Serialize, Deserialize)]
|
||||
#[allow(clippy::large_enum_variant)]
|
||||
pub enum PluginToHostV4 {
|
||||
Request { id: u64, payload: PluginRequest },
|
||||
Request {
|
||||
id: u64,
|
||||
tab_id: Option<u64>,
|
||||
payload: PluginRequest,
|
||||
},
|
||||
Response { id: u64, payload: HostResponse },
|
||||
Notification(NotificationEnvelope<PluginNotification>),
|
||||
}
|
||||
|
||||
@@ -27,7 +27,11 @@ macro_rules! vlog {
|
||||
/// A frame delivered from the V4 reader thread to the host main thread.
|
||||
pub enum HostIncoming {
|
||||
Response { id: u64, payload: crate::ipc::protocol::HostResponse },
|
||||
Request { id: u64, payload: Box<crate::ipc::protocol::PluginRequest> },
|
||||
Request {
|
||||
id: u64,
|
||||
tab_id: Option<u64>,
|
||||
payload: Box<crate::ipc::protocol::PluginRequest>,
|
||||
},
|
||||
}
|
||||
|
||||
/// Token-bucket rate limiter.
|
||||
@@ -117,9 +121,17 @@ pub fn run_host_reader_thread(
|
||||
}
|
||||
}
|
||||
}
|
||||
Ok(PluginToHostV4::Request { id, payload }) => {
|
||||
Ok(PluginToHostV4::Request {
|
||||
id,
|
||||
tab_id,
|
||||
payload,
|
||||
}) => {
|
||||
if incoming
|
||||
.send(HostIncoming::Request { id, payload: Box::new(payload) })
|
||||
.send(HostIncoming::Request {
|
||||
id,
|
||||
tab_id,
|
||||
payload: Box::new(payload),
|
||||
})
|
||||
.is_err()
|
||||
{
|
||||
break;
|
||||
|
||||
Reference in new issue
Block a user