On Tue, Jan 27, 2026 at 04:00:44PM +1300, Simon Glass wrote: Hi Simon, > Hi Quentin, > > On Tue, 27 Jan 2026 at 00:43, Quentin Schulz <[email protected]> wrote: > > > > Hi Simon, > > > > On 1/22/26 11:46 PM, Simon Glass wrote: > > > Hi, > > > > > > On Thu, 22 Jan 2026 at 02:06, Quentin Schulz <[email protected]> > > > wrote: > > >> > > >> Hi Wojciech, > > >> > > >> On 1/21/26 1:43 PM, Wojciech Dubowik wrote: > > >>> On Tue, Jan 20, 2026 at 04:53:04PM +0100, Quentin Schulz wrote: > > >>> Hello Quentin, > > >>>> Hi Wojciech, > > >>>> > > >>>> On 1/20/26 9:12 AM, Wojciech Dubowik wrote: > > >> [...] > > >>>>> + os.environ['SOFTHSM2_CONF'] = softhsm2_conf > > >>>> > > >>>> This is wrong, you'll be messing up with the environment of all tests > > >>>> being > > >>>> run in the same thread. You must use the "with > > >>>> unittest.mock.patch.dict('os.environ'," implementation I used in > > >>>> testFitSignPKCS11Simple. > > >>> > > >>> Well, I have done so in my V2 but has been commented as wrong by the > > >>> first reviewer. I will restore it back. > > >>> > > >> > > >> Indeed, I see Simon asked you to do this in v2 and I missed it. It isn't > > >> how we should be doing it. > > >> > > >> This is likely fine on its own because there's only one test that is now > > >> modifying os.environ's SOFTHSM2_CONF but this will be a problem next > > >> time a test wants to modify it. I actually hit this issue when > > >> developing the PKCS11 fit signing tests as I had two tests modifying the > > >> environment. > > >> > > >> The only trace of it left is the changelog in > > >> https://lore.kernel.org/u-boot/[email protected]/ > > >> > > >> """ > > >> - fixed issues due to modification of the environment in tests failing > > >> other tests, by using unittest.mock.patch.dict() on os.environ as > > >> suggested by the unittest.mock doc, > > >> """ > > >> > > >> and you can check the diff between the v2 and v3 to check I used to > > >> modify the env directly but now mock it instead. > > >> > > >> Sorry for not catching this, should have answered to Simon in the v2. > > > > > > In practice we try to set values for various which are important, so > > > future tests should explicitly delete the var if needed. But I am OK > > > > This is not working. See this very simple example (too lazy to use > > threading.Lock so synchronization done via time.sleep instead): > > > > """ > > #!/usr/bin/env python3 > > > > import os > > import time > > import threading > > > > > > def thread_func(n): > > if n == 1: > > time.sleep(1) > > print(f'Thread {n} read environ var FOO={os.environ["FOO"]}') > > if n == 1: > > time.sleep(1) > > print(f'Thread {n} set environ var FOO to foo{n}') > > os.environ['FOO'] = f'foo{n}' > > if n == 0: > > time.sleep(5) > > print(f'Thread {n} read environ var FOO={os.environ["FOO"]}') > > if n == 0: > > print(f'Thread {n} removes environ var FOO') > > del os.environ["FOO"] > > else: > > time.sleep(10) > > print(f'Thread {n} read environ var FOO={os.environ.get("FOO")}') > > > > > > threads = [] > > > > os.environ["FOO"] = "foo" > > > > for i in range(0, 2): > > t = threading.Thread(target=thread_func, args=(i,)) > > threads.append(t) > > > > for t in threads: > > t.start() > > > > for t in threads: > > t.join() > > > > """ > > > > This results in: > > > > """ > > Thread 0 read environ var FOO=foo > > Thread 0 set environ var FOO to foo0 > > Thread 1 read environ var FOO=foo0 > > Thread 1 set environ var FOO to foo1 > > Thread 1 read environ var FOO=foo1 > > Thread 0 read environ var FOO=foo1 > > Thread 0 removes environ var FOO > > Thread 1 read environ var FOO=None > > """ > > > > You see that modification made to os.environ in a different thread > > impacts the other threads. A test should definitely NOT modify anything > > for another test, especially not when it's already running. > > > > So now, I implemented mocking instead (like in my tests for PKCS11 in > > tools/binman/ftest.py) because I know it works. > > > > See: > > > > """ > > #!/usr/bin/env python3 > > > > import os > > import time > > import threading > > import unittest.mock > > > > > > def thread_func(n): > > if n == 1: > > time.sleep(1) > > print(f'Thread {n} read environ var FOO={os.environ.get("FOO")}') > > if n == 1: > > time.sleep(1) > > with unittest.mock.patch.dict('os.environ', > > {'FOO': f'foo{n}'}): > > print(f'Thread {n} set environ var FOO to foo{n}') > > if n == 0: > > time.sleep(5) > > print(f'Thread {n} read mocked environ var > > FOO={os.environ.get("FOO")}') > > if n == 1: > > time.sleep(6) > > print(f'Thread {n} read environ var FOO={os.environ.get("FOO")}') > > > > > > threads = [] > > > > for i in range(0, 2): > > t = threading.Thread(target=thread_func, args=(i,)) > > threads.append(t) > > > > for t in threads: > > t.start() > > > > for t in threads: > > t.join() > > """ > > > > Lo and behold, it.... does NOT work??????? > > > > I get: > > > > """ > > Thread 0 read environ var FOO=None > > Thread 0 set environ var FOO to foo0 > > Thread 1 read environ var FOO=foo0 > > Thread 1 set environ var FOO to foo1 > > Thread 1 read mocked environ var FOO=foo1 > > Thread 0 read mocked environ var FOO=foo1 > > Thread 0 read environ var FOO=None > > Thread 1 read environ var FOO=foo0 > > """ > > > > I've read that os.environ isn't thread-safe, due to setenv() in glibc > > not being thread-safe: > > https://sourceware.org/glibc/manual/latest/html_node/Environment-Access.html#Environment-Access-1 > > > > """ > > Modifications of environment variables are not allowed in multi-threaded > > programs. > > """ > > > > I'm not sure if this applies to any other Python implementation than > > CPython? But that is likely the one that most people are using. > > > > So... In short, I'm at a loss, no clue how to fix this (if it is even > > fixable). The obvious answer is "spawn multiple processes instead of > > multiple threads" but I can guarantee you, you don't want to be going > > this route as multiprocessing is a lot of headaches in Python. We could > > have the Python thread spawn a subprocess which has a different > > environment if we wanted to (via the `env` command for example), but > > that means not using binman Python API, rather its CLI. We could have > > bintools accept an environment dict that needs to be passed via the > > `env` command or the `env` kwargs of subprocess.Popen(). > > > > Headaches, headaches. > > I would argue that requiring a particular environment var for a test > is not a great idea. Better to pass an argument through the call stack > and have the external program run with a new environment containing > what is needed. Would it be ok to call softhsm utils with softhsm_conf like SOFTHSM2_CONF=... softhsm2-util? Then we don't have to mock environment. I would still have to set SOFTHSM install dir location with the env variable for mkeficapsule in binman generation. I guess it doesn't affect other tests as we don't need to change it.
Regards, Wojtek > > Regards, > Simon
