From f4498a4abf51054e14625422831d95fe8cbeab54 Mon Sep 17 00:00:00 2001 From: Philipp Oppermann Date: Thu, 13 Apr 2023 11:54:18 +0200 Subject: [PATCH 1/4] Map shared memory region as read-only in receiver This ensures that the data is not modified, even if shared with Python as zero-copy arrow array. Modifying the data could result in undefined behavior since there might be other subscribers that read the data at the same time. --- apis/rust/node/src/event_stream/event.rs | 1 + apis/rust/node/src/node/mod.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/apis/rust/node/src/event_stream/event.rs b/apis/rust/node/src/event_stream/event.rs index 120809a80..0c1591cf1 100644 --- a/apis/rust/node/src/event_stream/event.rs +++ b/apis/rust/node/src/event_stream/event.rs @@ -78,6 +78,7 @@ impl MappedInputData { let memory = Box::new( ShmemConf::new() .os_id(shared_memory_id) + .writable(false) .open() .wrap_err("failed to map shared memory input")?, ); diff --git a/apis/rust/node/src/node/mod.rs b/apis/rust/node/src/node/mod.rs index 5966d5d02..fc5c33fca 100644 --- a/apis/rust/node/src/node/mod.rs +++ b/apis/rust/node/src/node/mod.rs @@ -219,6 +219,7 @@ impl DoraNode { None => ShmemHandle(Box::new( ShmemConf::new() .size(data_len) + .writable(true) .create() .wrap_err("failed to allocate shared memory")?, )), From 5ba0366f6a4d64729783b3382c356579205aca4b Mon Sep 17 00:00:00 2001 From: Philipp Oppermann Date: Thu, 13 Apr 2023 11:55:10 +0200 Subject: [PATCH 2/4] Patch shared-memory dependency until PR is merged upstream --- Cargo.lock | 7 +++---- Cargo.toml | 4 ++++ 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index aa91ce425..7670b3dff 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4345,13 +4345,12 @@ dependencies = [ [[package]] name = "shared_memory" -version = "0.12.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ba8593196da75d9dc4f69349682bd4c2099f8cde114257d1ef7ef1b33d1aba54" +version = "0.12.5" +source = "git+https://github.com/phil-opp/shared_memory.git?branch=read-only#dac07946862bfe459425a86238c7251fd6ee3770" dependencies = [ "cfg-if", "libc", - "nix 0.23.2", + "nix 0.26.2", "rand", "win-sys", ] diff --git a/Cargo.toml b/Cargo.toml index c9552aeb1..5a980717b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -102,3 +102,7 @@ path = "examples/benchmark/run.rs" [[example]] name = "multiple-daemons" path = "examples/multiple-daemons/run.rs" + +[patch.crates-io.shared_memory] +git = "https://github.com/phil-opp/shared_memory.git" +branch = "read-only" From 979573689af70cdef73f39c3975212d5840685c6 Mon Sep 17 00:00:00 2001 From: Philipp Oppermann Date: Wed, 21 Jun 2023 10:34:46 +0200 Subject: [PATCH 3/4] Switch to forked `shared_memory_extended` crate The maintainer of the `shared_memory` crate did not react to our PR, so I decided to create a (temporary) fork. --- Cargo.lock | 11 ++++++----- Cargo.toml | 4 ---- apis/rust/node/Cargo.toml | 2 +- apis/rust/node/src/event_stream/event.rs | 2 +- apis/rust/node/src/node/mod.rs | 2 +- libraries/shared-memory-server/Cargo.toml | 2 +- libraries/shared-memory-server/src/channel.rs | 2 +- libraries/shared-memory-server/src/lib.rs | 2 +- 8 files changed, 12 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7670b3dff..21f7b78d7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1425,7 +1425,7 @@ dependencies = [ "serde_json", "serde_yaml 0.8.26", "shared-memory-server", - "shared_memory", + "shared_memory_extended", "thiserror", "tokio", "tracing", @@ -4339,14 +4339,15 @@ dependencies = [ "eyre", "raw_sync_2", "serde", - "shared_memory", + "shared_memory_extended", "tracing", ] [[package]] -name = "shared_memory" -version = "0.12.5" -source = "git+https://github.com/phil-opp/shared_memory.git?branch=read-only#dac07946862bfe459425a86238c7251fd6ee3770" +name = "shared_memory_extended" +version = "0.13.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "004d7ece9a3be64f85471d50967710b0a146144225bed5f0abd0514a3bed086f" dependencies = [ "cfg-if", "libc", diff --git a/Cargo.toml b/Cargo.toml index 5a980717b..c9552aeb1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -102,7 +102,3 @@ path = "examples/benchmark/run.rs" [[example]] name = "multiple-daemons" path = "examples/multiple-daemons/run.rs" - -[patch.crates-io.shared_memory] -git = "https://github.com/phil-opp/shared_memory.git" -branch = "read-only" diff --git a/apis/rust/node/Cargo.toml b/apis/rust/node/Cargo.toml index fc83aea2d..0d258bc9f 100644 --- a/apis/rust/node/Cargo.toml +++ b/apis/rust/node/Cargo.toml @@ -24,7 +24,7 @@ flume = "0.10.14" uuid = { version = "1.1.2", features = ["v4"] } capnp = "0.14.11" bincode = "1.3.3" -shared_memory = "0.12.0" +shared_memory_extended = "0.13.0" dora-tracing = { workspace = true, optional = true } arrow = "35.0.0" diff --git a/apis/rust/node/src/event_stream/event.rs b/apis/rust/node/src/event_stream/event.rs index 0c1591cf1..3c0a49ce1 100644 --- a/apis/rust/node/src/event_stream/event.rs +++ b/apis/rust/node/src/event_stream/event.rs @@ -5,7 +5,7 @@ use dora_core::{ message::Metadata, }; use eyre::Context; -use shared_memory::{Shmem, ShmemConf}; +use shared_memory_extended::{Shmem, ShmemConf}; #[derive(Debug)] #[non_exhaustive] diff --git a/apis/rust/node/src/node/mod.rs b/apis/rust/node/src/node/mod.rs index fc5c33fca..7245781fa 100644 --- a/apis/rust/node/src/node/mod.rs +++ b/apis/rust/node/src/node/mod.rs @@ -8,7 +8,7 @@ use dora_core::{ message::{uhlc, Metadata, MetadataParameters}, }; use eyre::{bail, WrapErr}; -use shared_memory::{Shmem, ShmemConf}; +use shared_memory_extended::{Shmem, ShmemConf}; use std::{ collections::{HashMap, VecDeque}, ops::{Deref, DerefMut}, diff --git a/libraries/shared-memory-server/Cargo.toml b/libraries/shared-memory-server/Cargo.toml index 167930dd8..fbca178cb 100644 --- a/libraries/shared-memory-server/Cargo.toml +++ b/libraries/shared-memory-server/Cargo.toml @@ -11,7 +11,7 @@ license.workspace = true [dependencies] eyre = "0.6.8" serde = { version = "1.0.152", features = ["derive"] } -shared_memory = "0.12.0" +shared_memory_extended = "0.13.0" # TODO use upstream release once https://github.com/elast0ny/raw_sync-rs/pull/29 is merged # Current fix, use personally pushed `raw_sync_2` version. raw_sync_2 = "0.1.5" diff --git a/libraries/shared-memory-server/src/channel.rs b/libraries/shared-memory-server/src/channel.rs index 4e6428baa..3f63c3c9f 100644 --- a/libraries/shared-memory-server/src/channel.rs +++ b/libraries/shared-memory-server/src/channel.rs @@ -1,7 +1,7 @@ use eyre::{eyre, Context}; use raw_sync_2::events::{Event, EventImpl, EventInit, EventState}; use serde::{Deserialize, Serialize}; -use shared_memory::Shmem; +use shared_memory_extended::Shmem; use std::{ mem, slice, sync::atomic::{AtomicBool, AtomicU64}, diff --git a/libraries/shared-memory-server/src/lib.rs b/libraries/shared-memory-server/src/lib.rs index e55dcb17d..32edbacaf 100644 --- a/libraries/shared-memory-server/src/lib.rs +++ b/libraries/shared-memory-server/src/lib.rs @@ -1,7 +1,7 @@ use self::channel::ShmemChannel; use eyre::{eyre, Context}; use serde::{Deserialize, Serialize}; -pub use shared_memory::{Shmem, ShmemConf}; +pub use shared_memory_extended::{Shmem, ShmemConf}; use std::marker::PhantomData; use std::time::Duration; From aee051284e66f2a23c2846efd3531b939ba0f46a Mon Sep 17 00:00:00 2001 From: Philipp Oppermann Date: Wed, 21 Jun 2023 12:24:19 +0200 Subject: [PATCH 4/4] Don't try to modify image in-place in python operator dataflow example The shared memory image must not be modified because it might be sent to other receivers too. Trying to modify it will result in a SEGFAULT now that we map the shared memory region as read-only. --- examples/python-operator-dataflow/plot.py | 1 + 1 file changed, 1 insertion(+) diff --git a/examples/python-operator-dataflow/plot.py b/examples/python-operator-dataflow/plot.py index 92f4cccce..f5d23ce71 100755 --- a/examples/python-operator-dataflow/plot.py +++ b/examples/python-operator-dataflow/plot.py @@ -58,6 +58,7 @@ def on_input( dora_input["value"] .to_numpy() .reshape((CAMERA_HEIGHT, CAMERA_WIDTH, 3)) + .copy() # copy the image because we want to modify it below ) self.image = frame