GNU bug report logs - #56487
xgselect race condition leading to abort when USE_GTK not defined

Previous Next

Full log

Message #8 received at 56487 <at> debbugs.gnu.org (full text, mbox):

From: Po Lu <luangruo <at> yahoo.com>
To: Tom Gillespie <tgbugs <at> gmail.com>
Cc: 56487 <at> debbugs.gnu.org
Subject: Re: bug#56487: xgselect race condition leading to abort when
 USE_GTK not defined
Date: Mon, 11 Jul 2022 09:53:22 +0800

Tom Gillespie <tgbugs <at> gmail.com> writes:

> From e120751b9de79be521f490f8c2f99597868b3208 Mon Sep 17 00:00:00 2001
> From: Tom Gillespie <tgbugs <at> 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.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.