[PATCH 03 of 10] rust-cpython: mark PySharedState as Sync so &'PySharedState can be Send

[Augie Fackler]

> On Sep 22, 2019, at 02:41, Yuya Nishihara <yuya at tcha.org> wrote:
> 
> # HG changeset patch
> # User Yuya Nishihara <yuya at tcha.org>
> # Date 1568674765 -32400
> #      Tue Sep 17 07:59:25 2019 +0900
> # Node ID fcaf01804ac8198d52d141d642d5ef7c935360cc
> # Parent  34a355c69f643f15ff46f4524c75137c08a8de3b
> rust-cpython: mark PySharedState as Sync so &'PySharedState can be Send

I asked some colleagues in the context  of D6859, airlifting my comments from phabricator:

I asked some coworkers, and they said:

> I do not believe this to be correct. An object is Sync if it is *inherently* thread-safe. This object is not -- the first thing borrow_mut does is a racy read of mutably_borrowed.

so I'm now leaning towards this being a bad idea.

Also, since it's possible we could call back into Python code and *that* could release-and-reacquire the GIL, the GIL guarding is probably insufficient to make this a "safe lie".

(I didn't look through the rest of this series in detail - this alone was enough for me to be sad and shuffle back to other things.)

> The goal is to store &'static PySharedState in $leaked struct, which allows
> us to move the $leaked struct out of the macro. Currently, it depends on
> inner.$data_member(py), which can't be generalized.
> 
> PySharedState is Sync because any mutation or read operation is synchronized
> by the Python GIL, py: Python<'a>, which should guarantee that &'PySharedState
> can be sent to another thread.
> 
> diff --git a/rust/hg-cpython/src/ref_sharing.rs b/rust/hg-cpython/src/ref_sharing.rs
> --- a/rust/hg-cpython/src/ref_sharing.rs
> +++ b/rust/hg-cpython/src/ref_sharing.rs
> @@ -18,6 +18,10 @@ pub struct PySharedState {
>     mutably_borrowed: Cell<bool>,
> }
> 
> +// &PySharedState can be Send because any access to inner cells is
> +// synchronized by the GIL.
> +unsafe impl Sync for PySharedState {}
> +
> impl PySharedState {
>     pub fn borrow_mut<'a, T>(
>         &'a self,
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel