bug#56487: xgselect race condition leading to abort when USE_GTK not defined

[Po Lu]

Tom Gillespie <tgbugs@gmail.com> writes:

> From e120751b9de79be521f490f8c2f99597868b3208 Mon Sep 17 00:00:00 2001
> From: Tom Gillespie <tgbugs@gmail.com>
> Date: Tue, 5 Jul 2022 13:43:25 -0700
> Subject: [PATCH] xgselect.c: avoid race condition leading to abort
>
> xgselect.c (xg_select): Remove ifdefs that were previously USE_GTK
> specific to be unconditional. This prevents a race condition caused by
> a call to release_select_lock from triggering when configuring Emacs
> with any toolkit other than gtk.
>
> xgselect.c (release_select_lock): Add a branch to check whether
> threads_holding_glib_lock has gone negative, and if so, restore to zero.
> In the case where there are multiple threads, threads_holding_glib_lock
> could become and stay negative, prevending g_main_context_release from
> ever being called, even when it should have been.
>
> As far as I can tell the way that the thread.c and xgselect.c code was
> written was with the intention of avoiding additional locks. This means
> that this code is exquisitely senstivie to slight changes in timing. A
> comment in thread.c has been added at one location where this happens.
>
> It is entirely possible that the removal of the ifdefs branching on
> USE_GTK resolves this issue by slightly changing the timings when
> using other/no toolkits so that the abort does not trigger. In all
> cases aborts can be induced by adding something like fputs in thread.c
> at the location of the new comment.
>
> For the record, the abort behavior is not present in Emacs 27, and was
> introduced by 9c62ffb08262c82b7e38e6eb5767f2087424aa47 (the bisect was
> quite a pain, so hopefully no one ever needs to do it again).

Thanks.  The commit message does not really comply with our style,
however.  The ChangeLog entries should look like this instead:


* src/xgselect.c (xg_select): Remove ifdefs that were previously
USE_GTK specific to be unconditional. This prevents a race
condition caused by a call to release_select_lock from
triggering when configuring Emacs with any toolkit other than
GTK.
(release_select_lock): Add a branch to check whether
threads_holding_glib_lock has gone negative, and if so, restore
to zero.  In the case where there are multiple threads,
threads_holding_glib_lock could become and stay negative,
prevending g_main_context_release from ever being called, even
when it should have been.


You can use C-c C-w inside a *vc-log* buffer to generate an
appropriately formatted commit message.