Repository: python/cpython
Pull request: gh-92452: Avoid race in initialization of sysconfig._CONFIG_VARS
Fixes #92452.

I couldn't figure out how to add a regression test, because the race only occurs while `sysconfig._CONFIG_VARS` is still being initialized by the first thread to call `sysconfig.get_config_vars`, and I don't see how to ensure that my test case (if I added one) is guaranteed to run first. I did check that the reproducer from the issue no longer asserts.

<!-- gh-issue-number: gh-92452 -->
* Issue: gh-92452
<!-- /gh-issue-number -->


Problem statement:
sysconfig.get_config_var is not thread-safe
### Reproducer

This program starts many threads racing to call `sysconfig.get_config_var`:
```
import sysconfig
from multiprocessing.pool import ThreadPool

def thread(i):
    assert sysconfig.get_config_var("srcdir") is not None

with ThreadPool() as pool:
    pool.map(thread, range(100))
```
When run with Python 3.11.0, this asserts as follows:
```
Traceback (most recent call last):
  File "/home/grees/core/junk.py", line 8, in <module>
    pool.map(thread, range(100))
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/grees/github.com/cpython/Lib/multiprocessing/pool.py", line 364, in map
    return self._map_async(func, iterable, mapstar, chunksize).get()
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/grees/github.com/cpython/Lib/multiprocessing/pool.py", line 771, in get
    raise self._value
    ^^^^^^^^^^^^^^^^^
  File "/home/grees/github.com/cpython/Lib/multiprocessing/pool.py", line 125, in worker
    result = (True, func(*args, **kwds))
                    ^^^^^^^^^^^^^^^^^^^
  File "/home/grees/github.com/cpython/Lib/multiprocessing/pool.py", line 48, in mapstar
    return list(map(*args))
           ^^^^^^^^^^^^^^^^
  File "/home/grees/core/junk.py", line 5, in thread
    assert sysconfig.get_config_var("srcdir") is not None
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError
```

### Analysis

In the `sysconfig` module, `get_config_var` calls `get_config_vars` which initializes the global variable `_CONFIG_VARS` in a thread-unsafe manner.

### Use case

The reproducer above is simplified from a multi-threaded build system. Each build thread needs to get some information from `sysconfig`, so that the threads are racing to be the first to call `get_config_vars`.

### Workaround

Adding an initial call to `get_config_vars` before starting any thread ensures that `_CONFIG_VARS` is reliably initialized.

Hints from issue discussion:

Commit message: Avoid race in initialization of sysconfig._CONFIG_VARS (GH-92452)

Patch to review:
diff --git a/Lib/sysconfig.py b/Lib/sysconfig.py
index e21b7303fecc6b..8155ad0cc6cbe8 100644
--- a/Lib/sysconfig.py
+++ b/Lib/sysconfig.py
@@ -2,6 +2,7 @@
 
 import os
 import sys
+import threading
 from os.path import pardir, realpath
 
 __all__ = [
@@ -172,7 +173,11 @@ def joinuser(*args):
 _BASE_PREFIX = os.path.normpath(sys.base_prefix)
 _EXEC_PREFIX = os.path.normpath(sys.exec_prefix)
 _BASE_EXEC_PREFIX = os.path.normpath(sys.base_exec_prefix)
+# Mutex guarding initialization of _CONFIG_VARS.
+_CONFIG_VARS_LOCK = threading.RLock();
 _CONFIG_VARS = None
+# True iff _CONFIG_VARS has been fully initialized.
+_CONFIG_VARS_INITIALIZED = False
 _USER_BASE = None
 
 # Regexes needed for parsing Makefile (and similar syntaxes,
@@ -635,8 +640,9 @@ def get_config_vars(*args):
     With arguments, return a list of values that result from looking up
     each argument in the configuration variable dictionary.
     """
-    global _CONFIG_VARS
-    if _CONFIG_VARS is None:
+
+    def init_config_vars():
+        global _CONFIG_VARS
         _CONFIG_VARS = {}
         # Normalized versions of prefix and exec_prefix are handy to have;
         # in fact, these are the standard versions used most places in the
@@ -696,6 +702,19 @@ def get_config_vars(*args):
             import _osx_support
             _osx_support.customize_config_vars(_CONFIG_VARS)
 
+        global _CONFIG_VARS_INITIALIZED
+        _CONFIG_VARS_INITIALIZED = True
+
+    # Avoid claiming the lock once initialization is complete.
+    if not _CONFIG_VARS_INITIALIZED:
+        with _CONFIG_VARS_LOCK:
+            # Test again with the lock held to avoid races. Note that
+            # we test _CONFIG_VARS here, not _CONFIG_VARS_INITIALIZED,
+            # to ensure that recursive calls to get_config_vars()
+            # don't re-enter init_config_vars().
+            if _CONFIG_VARS is None:
+                init_config_vars()
+
     if args:
         vals = []
         for name in args:
diff --git a/Misc/NEWS.d/next/Library/2022-05-08-08-47-32.gh-issue-92452.3pNHe6.rst b/Misc/NEWS.d/next/Library/2022-05-08-08-47-32.gh-issue-92452.3pNHe6.rst
new file mode 100644
index 00000000000000..25d6477aa9138a
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2022-05-08-08-47-32.gh-issue-92452.3pNHe6.rst
@@ -0,0 +1,2 @@
+Fixed a race condition that could cause :func:`sysconfig.get_config_var` to
+incorrectly return :const:`None` in multi-threaded programs.


Code review comments:
diff_hunk: @@ -635,8 +640,9 @@ def get_config_vars(*args):
     With arguments, return a list of values that result from looking up
     each argument in the configuration variable dictionary.
     """
-    global _CONFIG_VARS
-    if _CONFIG_VARS is None:
+
+    def init_config_vars():
path: Lib/sysconfig.py
text: @user1:
I think we can move this function outside, to decrease the complexity of this function.