Re: EXTERNAL - [PATCH v4 6/6] test: binman: Add test for pkcs11 signed capsule

[Wojciech Dubowik <Wojciech.Dubowik@mt.com>]

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 <quentin.schulz@cherry.de> wrote:
> >
> > Hi Simon,
> >
> > On 1/22/26 11:46 PM, Simon Glass wrote:
> > > Hi,
> > >
> > > On Thu, 22 Jan 2026 at 02:06, Quentin Schulz <quentin.schulz@cherry.de> 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/20251121-binman-engine-v3-0-b80180aaa783@cherry.de/
> > >>
> > >> """
> > >> - 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