bug#36609: 27.0.50; Possible race-condition in threading implementation

[dick.r.chiang@gmail.com]

Yes!  If you just add a negation to the __atomic_sub_fetch call, this passes
all the MREs.

It's not clear how you want to deal with the #else GNUC_PREREQ (4, 7, 0).

>>>>> "EZ" == Eli Zaretskii <eliz@gnu.org> writes:

>> From: dick.r.chiang@gmail.com
>> Cc: 36609@debbugs.gnu.org
>> Date: Sat, 19 Jun 2021 17:12:32 -0400
>>
>> "Suggest" or "Provide"?
>>
>> If the former, well, my recalcitrance should be your answer.
>>
>> If the latter, sure

EZ> Thanks, please try the patch below.

>> although if you've written the patch, then running my MREs would be epsilon
>> more effort.

EZ> You have a use case and wrote code with which you are familiar, so you are
EZ> in a better position to test it.  Moreover, I don't have access to a
EZ> system where these problems could happen, so it's far from epsilon effort
EZ> for me.

EZ> diff --git a/src/xgselect.c b/src/xgselect.c
EZ> index 0d91d55bad..9a90670b0f 100644
EZ> --- a/src/xgselect.c
EZ> +++ b/src/xgselect.c
EZ> @@ -34,12 +34,27 @@ static GMainContext *glib_main_context;

EZ>  void release_select_lock (void)
EZ>  {
EZ> +#if GNUC_PREREQ (4, 7, 0)
EZ> +  if (__atomic_sub_fetch (&threads_holding_glib_lock, 1, __ATOMIC_ACQ_REL))
EZ> +    g_main_context_release (glib_main_context);
EZ> +#else
EZ>    if (--threads_holding_glib_lock == 0)
EZ>      g_main_context_release (glib_main_context);
EZ> +#endif
EZ>  }

EZ>  static void acquire_select_lock (GMainContext *context)
EZ>  {
EZ> +#if GNUC_PREREQ (4, 7, 0)
EZ> +  if (__atomic_fetch_add (&threads_holding_glib_lock, 1, __ATOMIC_ACQ_REL) == 0)
EZ> +    {
EZ> +      glib_main_context = context;
EZ> +      while (!g_main_context_acquire (context))
EZ> +	{
EZ> +	  /* Spin. */
EZ> +	}
EZ> +    }
EZ> +#else
EZ>    if (threads_holding_glib_lock++ == 0)
EZ>      {
EZ>        glib_main_context = context;
EZ> @@ -48,6 +63,7 @@ static void acquire_select_lock (GMainContext *context)
EZ>  	  /* Spin. */
EZ>  	}
EZ>      }
EZ> +#endif
EZ>  }

EZ>  /* `xg_select' is a `pselect' replacement.  Why do we need a separate
EZ> function?