Use thread-local storage for the global Errors state to allow threaded builds. (GH-4507)

Distutils uses threading internally.

Also silence some warnings about redefined classes and function signatures when setting up the builtin scope. This is at most a second-best solution since we may not notice legitimate bugs on our side this way. Better make sure we have good test coverage of builtins and related optimisations.

Closes https://github.com/cython/cython/issues/4503

Cython/Compiler/Builtin.py

@@ -420,6 +420,7 @@ def init_builtin_structs():
 
 
 def init_builtins():
+    #Errors.init_thread()  # hopefully not needed - we should not emit warnings ourselves
     init_builtin_structs()
     init_builtin_types()
     init_builtin_funcs()

Cython/Compiler/Errors.py

@@ -12,6 +12,13 @@ except ImportError:
 import sys
 from contextlib import contextmanager
 
+try:
+    from threading import local as _threadlocal
+except ImportError:
+    class _threadlocal(object): pass
+
+threadlocal = _threadlocal()
+
 from ..Utils import open_new_file
 from . import DebugFlags
 from . import Options
@@ -120,35 +127,29 @@ class NoElementTreeInstalledException(PyrexError):
     implementation was found
     """
 
-listing_file = None
-num_errors = 0
-echo_file = None
-
-def open_listing_file(path, echo_to_stderr = 1):
+def open_listing_file(path, echo_to_stderr=True):
     # Begin a new error listing. If path is None, no file
     # is opened, the error counter is just reset.
-    global listing_file, num_errors, echo_file
     if path is not None:
-        listing_file = open_new_file(path)
+        threadlocal.cython_errors_listing_file = open_new_file(path)
     else:
-        listing_file = None
+        threadlocal.cython_errors_listing_file = None
     if echo_to_stderr:
-        echo_file = sys.stderr
+        threadlocal.cython_errors_echo_file = sys.stderr
     else:
-        echo_file = None
-    num_errors = 0
+        threadlocal.cython_errors_echo_file = None
+    threadlocal.cython_errors_count = 0
 
 def close_listing_file():
-    global listing_file
-    if listing_file:
-        listing_file.close()
-        listing_file = None
+    if threadlocal.cython_errors_listing_file:
+        threadlocal.cython_errors_listing_file.close()
+        threadlocal.cython_errors_listing_file = None
 
 def report_error(err, use_stack=True):
+    error_stack = threadlocal.cython_errors_stack
     if error_stack and use_stack:
         error_stack[-1].append(err)
     else:
-        global num_errors
         # See Main.py for why dual reporting occurs. Quick fix for now.
         if err.reported: return
         err.reported = True
@@ -157,15 +158,17 @@ def report_error(err, use_stack=True):
             # Python <= 2.5 does this for non-ASCII Unicode exceptions
             line = format_error(getattr(err, 'message_only', "[unprintable exception message]"),
                                 getattr(err, 'position', None)) + u'\n'
+        listing_file = threadlocal.cython_errors_listing_file
         if listing_file:
             try: listing_file.write(line)
             except UnicodeEncodeError:
                 listing_file.write(line.encode('ASCII', 'replace'))
+        echo_file = threadlocal.cython_errors_echo_file
         if echo_file:
             try: echo_file.write(line)
             except UnicodeEncodeError:
                 echo_file.write(line.encode('ASCII', 'replace'))
-        num_errors += 1
+        threadlocal.cython_errors_count += 1
         if Options.fast_fail:
             raise AbortError("fatal errors")
 
@@ -193,8 +196,10 @@ def message(position, message, level=1):
         return
     warn = CompileWarning(position, message)
     line = u"note: %s\n" % warn
+    listing_file = threadlocal.cython_errors_listing_file
     if listing_file:
         _write_file_encode(listing_file, line)
+    echo_file = threadlocal.cython_errors_echo_file
     if echo_file:
         _write_file_encode(echo_file, line)
     return warn
@@ -207,62 +212,75 @@ def warning(position, message, level=0):
         return error(position, message)
     warn = CompileWarning(position, message)
     line = u"warning: %s\n" % warn
+    listing_file = threadlocal.cython_errors_listing_file
     if listing_file:
         _write_file_encode(listing_file, line)
+    echo_file = threadlocal.cython_errors_echo_file
     if echo_file:
         _write_file_encode(echo_file, line)
     return warn
 
 
-_warn_once_seen = {}
 def warn_once(position, message, level=0):
-    if level < LEVEL or message in _warn_once_seen:
+    if level < LEVEL:
+        return
+    warn_once_seen = threadlocal.cython_errors_warn_once_seen
+    if message in warn_once_seen:
         return
     warn = CompileWarning(position, message)
     line = u"warning: %s\n" % warn
+    listing_file = threadlocal.cython_errors_listing_file
     if listing_file:
         _write_file_encode(listing_file, line)
+    echo_file = threadlocal.cython_errors_echo_file
     if echo_file:
         _write_file_encode(echo_file, line)
-    _warn_once_seen[message] = True
+    warn_once_seen[message] = True
     return warn
 
 
 # These functions can be used to momentarily suppress errors.
 
-error_stack = []
-
-
 def hold_errors():
-    error_stack.append([])
+    errors = []
+    threadlocal.cython_errors_stack.append(errors)
+    return errors
 
 
 def release_errors(ignore=False):
-    held_errors = error_stack.pop()
+    held_errors = threadlocal.cython_errors_stack.pop()
     if not ignore:
         for err in held_errors:
             report_error(err)
 
 
 def held_errors():
-    return error_stack[-1]
+    return threadlocal.cython_errors_stack[-1]
 
 
 # same as context manager:
 
 @contextmanager
 def local_errors(ignore=False):
-    errors = []
-    error_stack.append(errors)
+    errors = hold_errors()
     try:
         yield errors
     finally:
         release_errors(ignore=ignore)
 
 
-# this module needs a redesign to support parallel cythonisation, but
-# for now, the following works at least in sequential compiler runs
+# Keep all global state in thread local storage to support parallel cythonisation in distutils.
+
+def init_thread():
+    threadlocal.cython_errors_count = 0
+    threadlocal.cython_errors_listing_file = None
+    threadlocal.cython_errors_echo_file = None
+    threadlocal.cython_errors_warn_once_seen = set()
+    threadlocal.cython_errors_stack = []
 
 def reset():
-    _warn_once_seen.clear()
-    del error_stack[:]
+    threadlocal.cython_errors_warn_once_seen.clear()
+    del threadlocal.cython_errors_stack[:]
+
+def get_errors_count():
+    return threadlocal.cython_errors_count

Cython/Compiler/FusedNode.py

@@ -276,12 +276,12 @@ class FusedCFuncDefNode(StatListNode):
         Returns whether an error was issued and whether we should stop in
         in order to prevent a flood of errors.
         """
-        num_errors = Errors.num_errors
+        num_errors = Errors.get_errors_count()
         transform = ParseTreeTransforms.ReplaceFusedTypeChecks(
                                        copied_node.local_scope)
         transform(copied_node)
 
-        if Errors.num_errors > num_errors:
+        if Errors.get_errors_count() > num_errors:
             return False
 
         return True

Cython/Compiler/Main.py

@@ -337,7 +337,7 @@ class Context(object):
         source_filename = source_desc.filename
         scope.cpp = self.cpp
         # Parse the given source file and return a parse tree.
-        num_errors = Errors.num_errors
+        num_errors = Errors.get_errors_count()
         try:
             with Utils.open_source_file(source_filename) as f:
                 from . import Parsing
@@ -356,7 +356,7 @@ class Context(object):
             #traceback.print_exc()
             raise self._report_decode_error(source_desc, e)
 
-        if Errors.num_errors > num_errors:
+        if Errors.get_errors_count() > num_errors:
             raise CompileError()
         return tree
 
@@ -396,20 +396,19 @@ class Context(object):
         return ".".join(names)
 
     def setup_errors(self, options, result):
-        Errors.reset()  # clear any remaining error state
+        Errors.init_thread()
         if options.use_listing_file:
             path = result.listing_file = Utils.replace_suffix(result.main_source_file, ".lis")
         else:
             path = None
-        Errors.open_listing_file(path=path,
-                                 echo_to_stderr=options.errors_to_stderr)
+        Errors.open_listing_file(path=path, echo_to_stderr=options.errors_to_stderr)
 
     def teardown_errors(self, err, options, result):
         source_desc = result.compilation_source.source_desc
         if not isinstance(source_desc, FileSourceDescriptor):
             raise RuntimeError("Only file sources for code supported")
         Errors.close_listing_file()
-        result.num_errors = Errors.num_errors
+        result.num_errors = Errors.get_errors_count()
         if result.num_errors > 0:
             err = True
         if err and result.c_file:

Cython/Compiler/Pipeline.py

@@ -19,7 +19,7 @@ def dumptree(t):
 
 def abort_on_errors(node):
     # Stop the pipeline if there are any errors.
-    if Errors.num_errors != 0:
+    if Errors.get_errors_count() != 0:
         raise AbortError("pipeline break")
     return node
 
@@ -378,7 +378,7 @@ def run_pipeline(pipeline, source, printtree=True):
             error = err
     except InternalError as err:
         # Only raise if there was not an earlier error
-        if Errors.num_errors == 0:
+        if Errors.get_errors_count() == 0:
             raise
         error = err
     except AbortError as err:

Cython/Compiler/Symtab.py

@@ -485,7 +485,7 @@ class Scope(object):
             warning(pos, "'%s' is a reserved name in C." % cname, -1)
 
         entries = self.entries
-        if name and name in entries and not shadow:
+        if name and name in entries and not shadow and not self.is_builtin_scope:
             old_entry = entries[name]
 
             # Reject redeclared C++ functions only if they have the same type signature.
@@ -831,10 +831,10 @@ class Scope(object):
                 entry.type = entry.type.with_with_gil(type.with_gil)
             else:
                 if visibility == 'extern' and entry.visibility == 'extern':
-                    can_override = False
+                    can_override = self.is_builtin_scope
                     if self.is_cpp():
                         can_override = True
-                    elif cname:
+                    elif cname and not can_override:
                         # if all alternatives have different cnames,
                         # it's safe to allow signature overrides
                         for alt_entry in entry.all_alternatives():
@@ -1176,6 +1176,7 @@ class BuiltinScope(Scope):
     def builtin_scope(self):
         return self
 
+    # FIXME: remove redundancy with Builtin.builtin_types_table
     builtin_entries = {
 
         "type":   ["((PyObject*)&PyType_Type)", py_object_type],

Cython/TestUtils.py

@@ -51,13 +51,10 @@ def treetypes(root):
 class CythonTest(unittest.TestCase):
 
     def setUp(self):
-        self.listing_file = Errors.listing_file
-        self.echo_file = Errors.echo_file
-        Errors.listing_file = Errors.echo_file = None
+        Errors.init_thread()
 
     def tearDown(self):
-        Errors.listing_file = self.listing_file
-        Errors.echo_file = self.echo_file
+        Errors.init_thread()
 
     def assertLines(self, expected, result):
         "Checks that the given strings or lists of strings are equal line by line"

tests/run/test_fstring.pyx

@@ -17,7 +17,7 @@ if IS_PY2:
 
 from Cython.Build.Inline import cython_inline
 from Cython.TestUtils import CythonTest
-from Cython.Compiler.Errors import CompileError, hold_errors, release_errors, error_stack, held_errors
+from Cython.Compiler.Errors import CompileError, hold_errors, init_thread, held_errors
 
 def cy_eval(s, **kwargs):
     return cython_inline('return ' + s, force=True, **kwargs)
@@ -43,7 +43,7 @@ class TestCase(CythonTest):
                 else:
                     assert held_errors(), "Invalid Cython code failed to raise SyntaxError: %r" % str
                 finally:
-                    release_errors(ignore=True)
+                    init_thread()  # reset error status
             else:
                 try:
                     cython_inline(str, quiet=True)
@@ -52,8 +52,7 @@ class TestCase(CythonTest):
                 else:
                     assert False, "Invalid Cython code failed to raise %s: %r" % (exception_type, str)
                 finally:
-                    if error_stack:
-                        release_errors(ignore=True)
+                    init_thread()  # reset error status
 
     if IS_PY2:
         def assertEqual(self, first, second, msg=None):