GNU bug report logs - #63550
proced-refine-with-update-test is racy

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 63550 in the body.
You can then email your comments to 63550 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

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

From: Mattias Engdegård <mattias.engdegard <at> gmail.com>
To: Emacs Bug Report <bug-gnu-emacs <at> gnu.org>
Cc: Laurence Warne <laurencewarne <at> gmail.com>
Subject: proced-refine-with-update-test is racy
Date: Wed, 17 May 2023 11:38:22 +0200

The proced test `proced-refine-with-update-test` seems to have an update race which makes it fail randomly. (This is on macOS, but I see no reason it would be much different elsewhere.)
I'm marking it :unstable for now.

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

From: Basil Contovounesios <contovob <at> tcd.ie>
To: Mattias Engdegård <mattias.engdegard <at> gmail.com>
Cc: 63550 <at> debbugs.gnu.org, Laurence Warne <laurencewarne <at> gmail.com>
Subject: Re: bug#63550: proced-refine-with-update-test is racy
Date: Wed, 17 May 2023 12:44:25 +0100

Mattias Engdegård [2023-05-17 11:38 +0200] wrote:

> The proced test `proced-refine-with-update-test` seems to have an update race which makes it fail randomly. (This is on macOS, but I see no reason it would be much different elsewhere.)

Indeed I encounter the race from time to time on GNU/Linux as well.

> I'm marking it :unstable for now.

Thanks,

-- 
Basil

In GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.16.0, Xaw3d scroll bars) of 2023-05-15 built on blc
Repository revision: ebf5e4ca1cd39d3f23c4e37d9bdfeb2bf347df6d
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101004
System Description: Ubuntu 22.04.2 LTS

Configured using:
 'configure CC=gcc-12 'CFLAGS=-Og -ggdb3' --prefix=/home/bic/.local
 --with-file-notification=yes --with-x --with-x-toolkit=lucid'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY
INOTIFY PDUMPER PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS TREE_SITTER WEBP X11 XAW3D XDBE XIM XINPUT2 XPM
LUCID ZLIB

Important settings:
  value of $LC_MONETARY: en_IE.UTF-8
  value of $LC_NUMERIC: en_IE.UTF-8
  value of $LC_TIME: en_IE.UTF-8
  value of $LANG: en_GB.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

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

From: Laurence Warne <laurencewarne <at> gmail.com>
To: Basil Contovounesios <contovob <at> tcd.ie>
Cc: Mattias Engdegård <mattias.engdegard <at> gmail.com>,
 63550 <at> debbugs.gnu.org
Subject: Re: bug#63550: proced-refine-with-update-test is racy
Date: Sun, 21 May 2023 09:37:10 +0100

[Message part 1 (text/plain, inline)]

Hi,

> The proced test `proced-refine-with-update-test` seems to have an update
race which makes it fail randomly. (This is on macOS, but I see no reason
it would be much different elsewhere.)

Strange, I can't seem to reproduce running it continuously (I'm on linux
also).  Though whilst looking over the test suite, I don't think I like the
approach I took.  I've attached a new patch which changes the test suite to
mock the list of active processes (of course comments welcome).

Are either of you able to test whether this fixes the test in question?
The change also makes the tests a lot faster to run and hopefully less
system dependent.

Thanks, Laurence

[Message part 2 (text/html, inline)]

[0001-Mock-processes-in-proced-test.patch (text/x-patch, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Laurence Warne <laurencewarne <at> gmail.com>
Cc: contovob <at> tcd.ie, mattias.engdegard <at> gmail.com, 63550 <at> debbugs.gnu.org
Subject: Re: bug#63550: proced-refine-with-update-test is racy
Date: Sun, 21 May 2023 14:20:06 +0300

> Cc: Mattias Engdegård <mattias.engdegard <at> gmail.com>,
>  63550 <at> debbugs.gnu.org
> From: Laurence Warne <laurencewarne <at> gmail.com>
> Date: Sun, 21 May 2023 09:37:10 +0100
> 
> Strange, I can't seem to reproduce running it continuously (I'm on linux also).  Though whilst looking
> over the test suite, I don't think I like the approach I took.  I've attached a new patch which changes the
> test suite to mock the list of active processes (of course comments welcome).
> 
> Are either of you able to test whether this fixes the test in question?  The change also makes the tests
> a lot faster to run and hopefully less system dependent.

Is mocking out the real Proced display a good idea in this case?
These tests test the ability to manipulate real-life process-attribute
displays, so showing they work in synthetic environment verifies only
part of the functionality, no?

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

From: Laurence Warne <laurencewarne <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: contovob <at> tcd.ie, mattias.engdegard <at> gmail.com, 63550 <at> debbugs.gnu.org
Subject: Re: bug#63550: proced-refine-with-update-test is racy
Date: Sun, 21 May 2023 19:45:16 +0100

[Message part 1 (text/plain, inline)]

One thing I've noticed about the failing test is that we should probably
use `(proced-update)` instead of `(proced-update t)` so as not to refresh
`proced-process-alist` (I've attached a patch).  When I first saw this, I
thought this would fix the failure as I thought what might have been
happening was that the process used for the refinement might have exited
between proced being called, and then `(proced-update t)` being called, but
I think the test should still pass in this case (though I've optimistically
used 'fix' in the patch commit (: ).

Mattias (or Basil), are you able to provide a backtrace?

> Is mocking out the real Proced display a good idea in this case?
> These tests test the ability to manipulate real-life process-attribute
> displays, so showing they work in synthetic environment verifies only
> part of the functionality, no?

Since systems will likely have a variety of processes running at a time, I
think you are right, mocking will result in less coverage, but then again
the processes and their attributes people have running on their machines
are not consistent.  Perhaps we could use mocking, but only for features
which are difficult to test otherwise, like refinements?

Maybe I was too fast to jump to using mocking and it's tangential to this
issue, though I'd still be interested to see if the (mocking patch) fixes
the issue.

[Message part 2 (text/html, inline)]

[0001-Fix-unstable-proced-test.patch (text/x-patch, attachment)]

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

From: Mattias Engdegård <mattias.engdegard <at> gmail.com>
To: Laurence Warne <laurencewarne <at> gmail.com>
Cc: contovob <at> tcd.ie, Eli Zaretskii <eliz <at> gnu.org>, 63550 <at> debbugs.gnu.org
Subject: Re: bug#63550: proced-refine-with-update-test is racy
Date: Wed, 24 May 2023 09:56:22 +0200

21 maj 2023 kl. 20.45 skrev Laurence Warne <laurencewarne <at> gmail.com>:

> One thing I've noticed about the failing test is that we should probably use `(proced-update)` instead of `(proced-update t)` so as not to refresh `proced-process-alist` (I've attached a patch).  When I first saw this, I thought this would fix the failure as I thought what might have been happening was that the process used for the refinement might have exited between proced being called, and then `(proced-update t)` being called, but I think the test should still pass in this case (though I've optimistically used 'fix' in the patch commit (: ).

These steps seem to provoke a failure of the original test quite reliably on macOS:

1. add the call (sleep-for 1) both before and after (proced-refine), to widen the windows
2. in a different terminal, keep the command

  while true; do /bin/sleep 2 & sleep 0.1; done

running during the test, to create some churn in the process tables. (I'm using zsh if it matters.)

Changing (proced-update t) to (proced-update) does appear to remove the failure. Maybe you can decide whether it is better or not.

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

From: Laurence Warne <laurencewarne <at> gmail.com>
To: Mattias Engdegård <mattias.engdegard <at> gmail.com>
Cc: contovob <at> tcd.ie, Eli Zaretskii <eliz <at> gnu.org>, 63550 <at> debbugs.gnu.org
Subject: Re: bug#63550: proced-refine-with-update-test is racy
Date: Sat, 27 May 2023 20:14:34 +0100

[Message part 1 (text/plain, inline)]

> These steps seem to provoke a failure of the original test quite reliably
on macOS:

> 1. add the call (sleep-for 1) both before and after (proced-refine), to
widen the windows
> 2. in a different terminal, keep the command

>  while true; do /bin/sleep 2 & sleep 0.1; done

Thanks for this, with the help of these instructions I can reproduce.

> but I think the test should still pass in this case

Sorry I was wrong here, if the process used for the refinement exited
between proced being called, and then `(proced-update t)` being called,
then the whole process list will be shown as proced would skip applying the
refinement.  So I think using (proced-update) instead of (proced-update t)
is the simplest fix since it means `proced-process-alist` will never be
refreshed.  I've re-attached the patch from before, but with a bit more
explanation.

Thanks, Laurence

[Message part 2 (text/html, inline)]

[0001-Fix-unstable-proced-test.patch (text/x-patch, attachment)]

Message #28 received at 63550-done <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattias.engdegard <at> gmail.com>
To: Laurence Warne <laurencewarne <at> gmail.com>
Cc: Basil Contovounesios <contovob <at> tcd.ie>, Eli Zaretskii <eliz <at> gnu.org>,
 63550-done <at> debbugs.gnu.org
Subject: Re: bug#63550: proced-refine-with-update-test is racy
Date: Sun, 28 May 2023 13:39:16 +0200

27 maj 2023 kl. 21.14 skrev Laurence Warne <laurencewarne <at> gmail.com>:

> Sorry I was wrong here, if the process used for the refinement exited between proced being called, and then `(proced-update t)` being called, then the whole process list will be shown as proced would skip applying the refinement.  So I think using (proced-update) instead of (proced-update t) is the simplest fix since it means `proced-process-alist` will never be refreshed.  I've re-attached the patch from before, but with a bit more explanation.

Looks fine, applied and pushed and closing the bug. Thank you!
(I took the liberty to reflow the comments to fit within 80 columns.)

Previous Next

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