[libgpiod] Thread safety API contract

[libgpiod] Thread safety API contract

[Erik Schilling]

Hi all!

Currently it looks like libgpiod does not document any kind of thread
safety gurantee. However, the Python bindings tests
(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

Looking at the C implementations, it indeed looks? like this is a safe
thing to do - with the current implementation.

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?

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 :).

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

Re: [libgpiod] Thread safety API contract

[Bartosz Golaszewski]

On Wed, Sep 13, 2023 at 11:47 AM Erik Schilling
<erik.schilling@linaro.org> 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.

> (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.

> 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.

>
> 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.

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

Re: [libgpiod] Thread safety API contract

[Erik Schilling]

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@linaro.org> 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

Re: [libgpiod] Thread safety API contract

[Bartosz Golaszewski]

On Wed, Sep 13, 2023 at 3:36 PM Erik Schilling
<erik.schilling@linaro.org> wrote:
>
> 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@linaro.org> 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?
>

They can be used across threads alright. Thread-aware means: no global
state in the library, IOW two functions won't get in each other's way
unless they work on the same object.

Bart

Re: [libgpiod] Thread safety API contract

[Erik Schilling]

On Wed Sep 13, 2023 at 3:45 PM CEST, Bartosz Golaszewski wrote:
> On Wed, Sep 13, 2023 at 3:36 PM Erik Schilling
> <erik.schilling@linaro.org> wrote:
> >
> > 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@linaro.org> 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?
> >
>
> They can be used across threads alright. Thread-aware means: no global
> state in the library, IOW two functions won't get in each other's way
> unless they work on the same object.

Sorry, I did not phrase that question super well. A (hopefully) better
try:

If I create a chip and then open two line_requests from that single
chip. Can I use these two line_requests concurrently on different
threads? Or do both of them (and the chip) have to share a single lock?

My assumption was that everything derived from the same chip instance
must not run concurrently.

- Erik

Re: [libgpiod] Thread safety API contract

[Bartosz Golaszewski]

On Wed, Sep 13, 2023 at 4:10 PM Erik Schilling
<erik.schilling@linaro.org> wrote:
>
> On Wed Sep 13, 2023 at 3:45 PM CEST, Bartosz Golaszewski wrote:
> > On Wed, Sep 13, 2023 at 3:36 PM Erik Schilling
> > <erik.schilling@linaro.org> wrote:
> > >
> > > 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@linaro.org> 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?
> > >
> >
> > They can be used across threads alright. Thread-aware means: no global
> > state in the library, IOW two functions won't get in each other's way
> > unless they work on the same object.
>
> Sorry, I did not phrase that question super well. A (hopefully) better
> try:
>
> If I create a chip and then open two line_requests from that single
> chip. Can I use these two line_requests concurrently on different
> threads? Or do both of them (and the chip) have to share a single lock?
>
> My assumption was that everything derived from the same chip instance
> must not run concurrently.
>

Ah sorry, I didn't understand your question. Actually using requests
from a chip concurrently in a different thread is perfectly fine. The
two structures are independent from each other in user-space and their
work is synchronized in the kernel.

Bart

Re: [libgpiod] Thread safety API contract

[Erik Schilling]

On Wed Sep 13, 2023 at 5:17 PM CEST, Bartosz Golaszewski wrote:
> On Wed, Sep 13, 2023 at 4:10 PM Erik Schilling
> <erik.schilling@linaro.org> wrote:
> >
> > On Wed Sep 13, 2023 at 3:45 PM CEST, Bartosz Golaszewski wrote:
> > > On Wed, Sep 13, 2023 at 3:36 PM Erik Schilling
> > > <erik.schilling@linaro.org> wrote:
> > > >
> > > > 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@linaro.org> 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?
> > > >
> > >
> > > They can be used across threads alright. Thread-aware means: no global
> > > state in the library, IOW two functions won't get in each other's way
> > > unless they work on the same object.
> >
> > Sorry, I did not phrase that question super well. A (hopefully) better
> > try:
> >
> > If I create a chip and then open two line_requests from that single
> > chip. Can I use these two line_requests concurrently on different
> > threads? Or do both of them (and the chip) have to share a single lock?
> >
> > My assumption was that everything derived from the same chip instance
> > must not run concurrently.
> >
>
> Ah sorry, I didn't understand your question. Actually using requests
> from a chip concurrently in a different thread is perfectly fine. The
> two structures are independent from each other in user-space and their
> work is synchronized in the kernel.

Ah. That makes things a lot simpler. I think then we only need some
Send traits on the Rust structs. Does the same guarantee apply to all
structs that are "created" from a chip? Then I would look into whether I
can extend the docs while fixing the Rust bindings.

Thanks again!
- Erik

Re: [libgpiod] Thread safety API contract

[Erik Schilling]

On Wed Sep 13, 2023 at 10:10 PM CEST, Erik Schilling wrote:
> On Wed Sep 13, 2023 at 5:17 PM CEST, Bartosz Golaszewski wrote:
> > On Wed, Sep 13, 2023 at 4:10 PM Erik Schilling
> > <erik.schilling@linaro.org> wrote:
> > >
> > > On Wed Sep 13, 2023 at 3:45 PM CEST, Bartosz Golaszewski wrote:
> > > > On Wed, Sep 13, 2023 at 3:36 PM Erik Schilling
> > > > <erik.schilling@linaro.org> wrote:
> > > > >
> > > > > 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@linaro.org> 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?
> > > > >
> > > >
> > > > They can be used across threads alright. Thread-aware means: no global
> > > > state in the library, IOW two functions won't get in each other's way
> > > > unless they work on the same object.
> > >
> > > Sorry, I did not phrase that question super well. A (hopefully) better
> > > try:
> > >
> > > If I create a chip and then open two line_requests from that single
> > > chip. Can I use these two line_requests concurrently on different
> > > threads? Or do both of them (and the chip) have to share a single lock?
> > >
> > > My assumption was that everything derived from the same chip instance
> > > must not run concurrently.
> > >
> >
> > Ah sorry, I didn't understand your question. Actually using requests
> > from a chip concurrently in a different thread is perfectly fine. The
> > two structures are independent from each other in user-space and their
> > work is synchronized in the kernel.
>
> Ah. That makes things a lot simpler. I think then we only need some
> Send traits on the Rust structs. Does the same guarantee apply to all
> structs that are "created" from a chip? Then I would look into whether I
> can extend the docs while fixing the Rust bindings.

Summarizing a short discussion between Bart an me:

- objects created from chips are standalone and different instances can
  be used concurrently
- exception: edge_events from buffers are tied to the buffer unless one
  copies them (as stated in the docs).

Will send a patch mentioning this in the docs and adjusting the Rust
bindings.

- Erik