Re: [libgpiod] Thread safety API contract

["Erik Schilling" <erik.schilling@xxxxxxxxxx>]

On Wed Sep 13, 2023 at 2:03 PM CEST, Bartosz Golaszewski wrote:
> On Wed, Sep 13, 2023 at 11:47 AM Erik Schilling
> <erik.schilling@xxxxxxxxxx> wrote:
> >
> > Hi all!
> >
> > Currently it looks like libgpiod does not document any kind of thread
> > safety gurantee. However, the Python bindings tests
>
> Indeed, the library is thread-aware but not thread-safe. Just like
> what is recommended for low-level system libraries.

Just to confirm:

I assume this means: thread-aware in the sense that all created objects
(chips, line_requests, ...) together may only be used by a single thread
at once? So line_requests of a same chip may not be used across threads?

> > (test_request_reconfigure_release_events) are using sequences like this:
> >
> > Thread 1 creates chip + some watches
> > Thread 1 creates Thread 2
> > Thread 2 issues a request_lines on the chip
> > Thread 2 reconfigures the line direction
> > Thread 1 joins Thread 2
> > Thread 1 closes the chip
> >
> > Implicitly this depends on a couple guarantees:
> > 1. Calling chip-related functions does not require synchronisation
> >    primitives (other than keeping the chip open).
> >    -> wait_info_event, read_info_event and request_lines are called
> >       concurrently
> > 2. Requests may be modified by other threads
> >    -> at least reconfiguring the direction is done
> >
>
> Well, this is just a test-case that's meant to trigger a line state
> event. Now that you're mentioning this, it does look like I should
> have used an entirely separate chip object. Good catch!
>
> > Looking at the C implementations, it indeed looks? like this is a safe
> > thing to do - with the current implementation.
> >
>
> No it isn't. That is: maybe it is but it's not on purpose. There are
> no thread-safety guarantees.

Right. Thats what I was trying to suggest with "- with the current
implementation" suffix.

> > My question is: Is this an intentional gurantee that will be guranteed
> > in future releases? I am trying to figure out whether the current
> > contract exposed by the Rust bindings is correct and/or may need to
> > be extended. So which guarantees are provided by the current and future
> > C lib?
>
> None. Except reentrancy for all functions.

Thanks for clarifying!

> > Currently, the Rust bindings are advertising that the chip may be `Send`
> > to other threads. This means one thread may forget about it and another
> > thread receives it. In contrast, a request for a line is currently not
> > allowed to be transferred to other threads (it is missing the `Send`
> > marker).
> >
> > While in C and C++ thread-safety is typically not enforced by the
> > compiler, Rust has mechanisms to do this. But I would like to document
> > the C lib's situation before inventing rules for the Rust bindings :).
> >
>
> I cannot help you with that but whatever rust does, it needs to keep
> in mind the C objects need to be synchronized as they offer no
> guarantees.

I will think of something in a calm moment :). I think we may need to
prevent the chip from being moved to other threads while leaving child
objects behind.

Thanks
- Erik

>
> Bartosz
>
> > Trigger of my question was that we glossed over these details in
> > vhost-device-gpio:
> >
> > https://github.com/rust-vmm/vhost-device/pull/435#issuecomment-1717205620
> >
> > - Erik