From: Quentin Schulz <quentin.schulz@cherry.de>
To: richard.purdie@linuxfoundation.org,
openembedded-core@lists.openembedded.org
Subject: Re: [OE-core] [PATCH] oeqa/selftest: Fix single threaded race issue
Date: Mon, 6 Oct 2025 18:56:15 +0200 [thread overview]
Message-ID: <ff5c5818-a596-4f5b-8a50-dfda83a3664e@cherry.de> (raw)
In-Reply-To: <20251006130902.846106-1-richard.purdie@linuxfoundation.org>
Hi Richard,
On 10/6/25 3:09 PM, Richard Purdie via lists.openembedded.org wrote:
> oe-selftest sets up separate build directories to run the tests in.
> To to this, environment paths pointing at the previous build directory
> are updated. In the multi-threaded case this is fine as the thread is
> destroyed and the parent remains unchanged but in the single threaded
> case, the environment is broken afterwards. This can mean we try and access
> a directory which is in the process of being deleted (e.g. by clobberdir).
>
> Restore the environment afterwards regardless to ensure the single threaded
> case doesn't try and access the build directory which is now being deleted.
>
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
> meta/lib/oeqa/selftest/context.py | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/meta/lib/oeqa/selftest/context.py b/meta/lib/oeqa/selftest/context.py
> index 16f82c6737d..c9eb4817253 100644
> --- a/meta/lib/oeqa/selftest/context.py
> +++ b/meta/lib/oeqa/selftest/context.py
> @@ -44,9 +44,13 @@ class NonConcurrentTestSuite(unittest.TestSuite):
> self.bb_vars = bb_vars
>
> def run(self, result):
> + origenv = os.environ.copy()
> (builddir, newbuilddir) = self.setupfunc("-st", None, self.suite)
> ret = super().run(result)
> + # In forks we don't have to restore but in a single process, restore cwd and the env
> os.chdir(builddir)
> + for e in origenv:
> + os.environ[e] = origenv[e]
Wondering if we cannot replace the loop with
os.environ.update(origenv)
which should have the same behavior?
Note that keys that didn't exist in origenv but exists in os.environ
after the copy() will stay there (in the update() and suggested implem
in the patch). If you want to really have the same one, I'm wondering if
os.environ = origenv[:] would work, but I'm assuming there's a reason
you didn't go for that that I am missing. Another option could be to
delete/pop all keys that do not exist in both, e.g. with (not tested)
del_keys = set(os.environ.keys()) - set(origenv.keys())
for del_key in del_keys:
del os.environ[del_key]
Though I'm wondering if we cannot simply call os.environ.clear() before?
Not sure what the side effects could be, I have a feeling this
os.environ is a bit more than a dict :)
Cheers,
Quentin
next prev parent reply other threads:[~2025-10-06 16:56 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-06 13:09 [PATCH] oeqa/selftest: Fix single threaded race issue Richard Purdie
2025-10-06 16:56 ` Quentin Schulz [this message]
2025-10-06 23:12 ` [OE-core] " Richard Purdie
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ff5c5818-a596-4f5b-8a50-dfda83a3664e@cherry.de \
--to=quentin.schulz@cherry.de \
--cc=openembedded-core@lists.openembedded.org \
--cc=richard.purdie@linuxfoundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox