GNU bug report logs - #79334
[PATCH] Don't release thread select lock unnecessarily

Previous Next

Package: emacs;

Reported by: Spencer Baugh <sbaugh <at> janestreet.com>

Date: Thu, 28 Aug 2025 21:10:02 UTC

Severity: normal

Tags: patch

Full log


View this message in rfc822 format

From: Eli Zaretskii <eliz <at> gnu.org>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: 79334 <at> debbugs.gnu.org, eggert <at> cs.ucla.edu, dmitry <at> gutov.dev
Subject: bug#79334: [PATCH] Don't release thread select lock unnecessarily, [PATCH] Defer closing file descriptors used by other threads
Date: Sat, 30 Aug 2025 09:35:42 +0300
> From: Spencer Baugh <sbaugh <at> janestreet.com>
> Cc: 79334 <at> debbugs.gnu.org,  eggert <at> cs.ucla.edu,  dmitry <at> gutov.dev
> Date: Fri, 29 Aug 2025 16:25:39 -0400
> 
> In related news: running process-tests.el in a loop also triggers my
> "Closing deferred fd" message.  Which suggests that src/process-tests.el
> was probably also suffering from bugs in this area, which could have
> been causing hangs or flaky failures.  Which should now hopefully be
> fixed.

process-tests uses questionable setup, apart of being based on
non-portable assumptions.  At the time, I was unable to convince the
author of the tests to spell out the assumptions and expectations from
what should happen in those situations, without which it was
impossible to decide whether some failures on MS-Windows are bugs or
just non-portable code that cannot work on Windows.  Some of those
tests always fail for me on Windows.

> +      for (int fd = 0; fd <= max_desc; ++fd)
> +	{
> +	  if (fd_callback_info[fd].waiting_thread == current_thread
> +	      && (fd_callback_info[fd].flags == WAITING_FOR_CLOSE_FD))
                                             ^^^^^^^^^^^^^^^^^^^^^^^
Shouldn't this test that the WAITING_FOR_CLOSE_FD bit is set, not that
it is the _only_ bit set?  Even if the code is correct today, it might
not be future-proof if we add some additional flags at some point.

> +	      /* Ignore any events which happened on this fd. */
> +	      FD_CLR (fd, &Available);
> +	      FD_CLR (fd, &Writeok);

Is this wise?  How can we be sure that these events were already
handled (presumably, in another thread)?  If they were not, we will
lose events.  What will happen if we don't ignore them here?




This bug report was last modified 8 days ago.

Previous Next


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