Hi Wojciech, On Tue, 3 Feb 2026 at 21:18, Wojciech Dubowik <[email protected]> wrote: > > 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. >
So long as it works it seems fine to me. Regards, Simon
