EXTERNAL - [PATCH v4 6/6] test: binman: Add test for pkcs11 signed capsule
Quentin Schulz
quentin.schulz at cherry.de
Wed Feb 11 17:09:43 CET 2026
Hi Wojciech, Simon,
On 2/4/26 10:49 AM, Quentin Schulz wrote:
> Hi Wojciech,
>
> On 2/3/26 9:17 AM, Wojciech Dubowik 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
>>> <quentin.schulz at 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 at 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://eur02.safelinks.protection.outlook.com/?
>>>>>> url=https%3A%2F%2Flore.kernel.org%2Fu-boot%2F20251121-binman-
>>>>>> engine-v3-0-
>>>>>> b80180aaa783%40cherry.de%2F&data=05%7C02%7Cquentin.schulz%40cherry.de%7C4be38001079e40572d5708de63d2c7a6%7C5e0e1b5221b54e7b83bb514ec460677e%7C0%7C0%7C639057954135737078%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=B5%2Bx5iUO54EpovAyDBUCwuq5NgcfcejIoD6Unhdoa%2FM%3D&reserved=0
>>>>>>
>>>>>> """
>>>>>> - 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://eur02.safelinks.protection.outlook.com/?
>>>> url=https%3A%2F%2Fsourceware.org%2Fglibc%2Fmanual%2Flatest%2Fhtml_node%2FEnvironment-Access.html%23Environment-Access-1&data=05%7C02%7Cquentin.schulz%40cherry.de%7C4be38001079e40572d5708de63d2c7a6%7C5e0e1b5221b54e7b83bb514ec460677e%7C0%7C0%7C639057954135756872%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=K5LZPB7BkLnUsLU%2BFFK4Bmp1vOXqF8WiSryObldGBAY%3D&reserved=0
>>>>
>>>> """
>>>> 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
We are already doing this actually and this is what I was missing.
When using binman in parallel (the default when not passing -P1), we
create a ConcurrentTestSuite whose make_tests function "pointer"
parameter is fork_for_tests() from concurrencytest. What this does is
collect all tests to run from all test suites, divide them equally to
the number of processes then fork the main process that many times.
Before forking, concurrencytest creates a pipe for communication between
the child process and main process. The main process keeps track of all
those pipes and generate a TestCase list (one per child process) that
ConcurrentTestSuite then consumes. This will then create a thread per
TestCase. This essentially means that:
- tests are run in different processes,
- but each process runs multiple tests sequentially (not in parallel,
not in separate threads),
- threads are used to get the output of tests from a different process
but that's just implementation details,
As such, we don't have to care about thread-safety as that is only used
by concurrencytest for communication purposes.
However, this does mean that we absolutely need to clean up after
ourselves. This is the reason why modifications to os.environ were
breaking stuff in other tests for me, not because of thread-unsafe code,
but because sequential tests inherit the dirty laundry from the previous
test and I didn't clean things up properly. This explains why using
unittest.mock.patch() actually fixed things up, because it's handled by
the context manager anyway.
So we have the option of using either a contextmanager
(unittest.mock.patch) for setting the environment and not having to care
about cleaning it up afterwards. Or have the code snippet setting the
environment and what uses it in a big try:finally which then cleans up
before leaving the test.
I don't think propagating the env through Binman (see _DoBinman() which
runs binman like a Python module thus inheriting the current
environment) will make things easier or more readable, so please ignore
the rest of the mail I answer to.
The TL;DR is: no thread-safety issue in tools/binman because we don't
use threads. Use unittest.mock.patch contextmanager, or write to
os.environ and undo it before exiting the test (and this needs to be
done in a try:finally block to avoid exiting early).
Cheers,
Quentin
More information about the U-Boot
mailing list