From: Erik Schilling <erik.schilling@linaro.org>
To: Linux-GPIO <linux-gpio@vger.kernel.org>
Cc: Bartosz Golaszewski <brgl@bgdev.pl>,
Viresh Kumar <viresh.kumar@linaro.org>,
Erik Schilling <erik.schilling@linaro.org>
Subject: [libgpiod][PATCH 3/3] bindings: rust: mark all owning types as `Send`
Date: Thu, 28 Sep 2023 16:37:30 +0200 [thread overview]
Message-ID: <20230928-rust-send-trait-v1-3-30b4f59d13cb@linaro.org> (raw)
In-Reply-To: <20230928-rust-send-trait-v1-0-30b4f59d13cb@linaro.org>
The thread-safety rules of libgpiod allow individual object instances to
be used from different threads. So far, this was not actually possible
with the Rust bindings. Not being `Send` disallowed the user to transfer
the ownership to different threads.
Rust also has a `Sync` marker. That one would even allow sending
references of objects to other threads. Since we wrap a lot of C
functions with `fn foo(&self)` signatures, that would not be safe.
libgpiod does not allow concurrent API calls to the same object instance
- which Rust would allow for read-only references. Thus, we do not
define that one.
Chip was already modeled correctly.
line::Info is not marked as Send since it may either be owning or non-
owning. That problem is fixed as part of a separate pull request [1].
[1] https://lore.kernel.org/r/20230927-rust-line-info-soundness-v1-0-990dce6f18ab@linaro.org
Link: https://lore.kernel.org/r/CVHO091CC80Y.3KUOSLSOBVL0T@ablu-work
Signed-off-by: Erik Schilling <erik.schilling@linaro.org>
---
bindings/rust/libgpiod/src/edge_event.rs | 4 ++++
bindings/rust/libgpiod/src/event_buffer.rs | 8 ++++++++
bindings/rust/libgpiod/src/info_event.rs | 4 ++++
bindings/rust/libgpiod/src/line_config.rs | 4 ++++
bindings/rust/libgpiod/src/line_request.rs | 4 ++++
bindings/rust/libgpiod/src/line_settings.rs | 4 ++++
bindings/rust/libgpiod/src/request_config.rs | 4 ++++
7 files changed, 32 insertions(+)
diff --git a/bindings/rust/libgpiod/src/edge_event.rs b/bindings/rust/libgpiod/src/edge_event.rs
index 0c0cfbc..639f033 100644
--- a/bindings/rust/libgpiod/src/edge_event.rs
+++ b/bindings/rust/libgpiod/src/edge_event.rs
@@ -24,6 +24,10 @@ use super::{
#[derive(Debug, Eq, PartialEq)]
pub struct Event(*mut gpiod::gpiod_edge_event);
+// SAFETY: Event models a wrapper around an owned gpiod_edge_event and may
+// be safely sent to other threads.
+unsafe impl Send for Event {}
+
impl Event {
pub fn event_clone(event: &Event) -> Result<Event> {
// SAFETY: `gpiod_edge_event` is guaranteed to be valid here.
diff --git a/bindings/rust/libgpiod/src/event_buffer.rs b/bindings/rust/libgpiod/src/event_buffer.rs
index 2e4bfd3..68d6e2f 100644
--- a/bindings/rust/libgpiod/src/event_buffer.rs
+++ b/bindings/rust/libgpiod/src/event_buffer.rs
@@ -68,6 +68,14 @@ pub struct Buffer {
events: Vec<*mut gpiod::gpiod_edge_event>,
}
+// SAFETY: Buffer models an owned gpiod_edge_event_buffer. However, there may
+// be events tied to it. Concurrent access from multiple threads to a buffer
+// and its associated events is not allowed by the C lib.
+// In Rust, those events will always be borrowed from a buffer instance. Thus,
+// either Rust prevents the user to move the Buffer while there are still
+// borrowed events, or we can safely send the the Buffer.
+unsafe impl Send for Buffer {}
+
impl Buffer {
/// Create a new edge event buffer.
///
diff --git a/bindings/rust/libgpiod/src/info_event.rs b/bindings/rust/libgpiod/src/info_event.rs
index db60600..1e26b37 100644
--- a/bindings/rust/libgpiod/src/info_event.rs
+++ b/bindings/rust/libgpiod/src/info_event.rs
@@ -25,6 +25,10 @@ pub struct Event {
pub(crate) event: *mut gpiod::gpiod_info_event,
}
+// SAFETY: Event models a wrapper around an owned gpiod_info_event and may be
+// safely sent to other threads.
+unsafe impl Send for Event {}
+
impl Event {
/// Get a single chip's line's status change event.
pub(crate) fn new(event: *mut gpiod::gpiod_info_event) -> Self {
diff --git a/bindings/rust/libgpiod/src/line_config.rs b/bindings/rust/libgpiod/src/line_config.rs
index f4f03f1..d0a4aba 100644
--- a/bindings/rust/libgpiod/src/line_config.rs
+++ b/bindings/rust/libgpiod/src/line_config.rs
@@ -28,6 +28,10 @@ pub struct Config {
pub(crate) config: *mut gpiod::gpiod_line_config,
}
+// SAFETY: Config models a wrapper around an owned gpiod_line_config and may be
+// safely sent to other threads.
+unsafe impl Send for Config {}
+
impl Config {
/// Create a new line config object.
pub fn new() -> Result<Self> {
diff --git a/bindings/rust/libgpiod/src/line_request.rs b/bindings/rust/libgpiod/src/line_request.rs
index a5697d6..64ef05d 100644
--- a/bindings/rust/libgpiod/src/line_request.rs
+++ b/bindings/rust/libgpiod/src/line_request.rs
@@ -20,6 +20,10 @@ pub struct Request {
pub(crate) request: *mut gpiod::gpiod_line_request,
}
+// SAFETY: Request models a wrapper around an owned gpiod_line_request and may
+// be safely sent to other threads.
+unsafe impl Send for Request {}
+
impl Request {
/// Request a set of lines for exclusive usage.
pub(crate) fn new(request: *mut gpiod::gpiod_line_request) -> Result<Self> {
diff --git a/bindings/rust/libgpiod/src/line_settings.rs b/bindings/rust/libgpiod/src/line_settings.rs
index f0b3e9c..c81d118 100644
--- a/bindings/rust/libgpiod/src/line_settings.rs
+++ b/bindings/rust/libgpiod/src/line_settings.rs
@@ -25,6 +25,10 @@ pub struct Settings {
pub(crate) settings: *mut gpiod::gpiod_line_settings,
}
+// SAFETY: Settings models a wrapper around an owned gpiod_line_settings and may
+// be safely sent to other threads.
+unsafe impl Send for Settings {}
+
impl Settings {
/// Create a new line settings object.
pub fn new() -> Result<Self> {
diff --git a/bindings/rust/libgpiod/src/request_config.rs b/bindings/rust/libgpiod/src/request_config.rs
index 5bde7c6..9b66cc9 100644
--- a/bindings/rust/libgpiod/src/request_config.rs
+++ b/bindings/rust/libgpiod/src/request_config.rs
@@ -20,6 +20,10 @@ pub struct Config {
pub(crate) config: *mut gpiod::gpiod_request_config,
}
+// SAFETY: Config models a wrapper around an owned gpiod_request_config and may
+// be safely sent to other threads.
+unsafe impl Send for Config {}
+
impl Config {
/// Create a new request config object.
pub fn new() -> Result<Self> {
--
2.41.0
next prev parent reply other threads:[~2023-09-28 14:37 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-28 14:37 [libgpiod][PATCH 0/3] thread-safety doc + Rust modeling Erik Schilling
2023-09-28 14:37 ` [libgpiod][PATCH 1/3] doc: drop unneeded <p> tags Erik Schilling
2023-09-28 14:37 ` [libgpiod][PATCH 2/3] doc: document thread safety guarantees Erik Schilling
2023-09-28 14:37 ` Erik Schilling [this message]
2023-09-29 10:58 ` [libgpiod][PATCH 3/3] bindings: rust: mark all owning types as `Send` Viresh Kumar
2023-09-29 12:45 ` Bartosz Golaszewski
2023-09-29 12:47 ` Erik Schilling
2023-09-29 13:07 ` Bartosz Golaszewski
2023-10-02 7:16 ` [libgpiod][PATCH 0/3] thread-safety doc + Rust modeling Bartosz Golaszewski
2023-10-02 7:22 ` Erik Schilling
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230928-rust-send-trait-v1-3-30b4f59d13cb@linaro.org \
--to=erik.schilling@linaro.org \
--cc=brgl@bgdev.pl \
--cc=linux-gpio@vger.kernel.org \
--cc=viresh.kumar@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).