[WIP] Fix race condition when importing typelibs on Windows

CHANGES.txt

@@ -35,6 +35,11 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
       User-facing behavior does not change with this fix (GH Issue #3726).
     - Fix occasional test failures caused by not being able to find a file or directory fixture
       when running multiple tests with multiple jobs.
+    - Fix MSVC build failures when compiling multiple files from the same directory that
+      all use `#import` to import the same typelibs (GH Issue #2161). To fix this, SCons now
+      supports a new parameter to the `SideEffect` function: `temporary`. This parameter
+      should only be set to `True` in special cases where a build tool performs a one-time
+      one-time generation of a side effect and then leaves it in place for the duration of the build.
 
   From Joachim Kuebart:
     - Suppress missing SConscript deprecation warning if `must_exist=False`

SCons/Environment.py

@@ -2243,7 +2243,7 @@ def SConsignFile(self, name=".sconsign", dbm_module=None):
                 self.Execute(SCons.Defaults.Mkdir(sconsign_dir))
         SCons.SConsign.File(name, dbm_module)
 
-    def SideEffect(self, side_effect, target):
+    def SideEffect(self, side_effect, target, temporary=False):
         """Tell scons that side_effects are built as side
         effects of building targets."""
         side_effects = self.arg2nodes(side_effect, self.fs.Entry)
@@ -2252,8 +2252,16 @@ def SideEffect(self, side_effect, target):
         for side_effect in side_effects:
             if side_effect.multiple_side_effect_has_builder():
                 raise UserError("Multiple ways to build the same target were specified for: %s" % str(side_effect))
+
+            if temporary and side_effect.is_up_to_date():
+                # Only add a temporary side effect if it is not already
+                # up-to-date. Otherwise, we can consider the temporary
+                # side effect to not be necessary.
+                continue
+
             side_effect.add_source(targets)
             side_effect.side_effect = 1
+            side_effect.side_effect_temporary = temporary and 1 or 0
             self.Precious(side_effect)
             for target in targets:
                 target.side_effects.append(side_effect)

SCons/Environment.xml

@@ -2839,7 +2839,7 @@ if 'FOO' not in env: env['FOO'] = 'foo'
 
 <scons_function name="SideEffect">
 <arguments>
-(side_effect, target)
+(side_effect, target, temporary=False)
 </arguments>
 <summary>
 <para>
@@ -2897,6 +2897,19 @@ or
 &f-env-Clean;
 function.
 </para>
+
+<para>
+If <parameter>temporary</parameter> is set to
+<option>True</option>, the side effect is removed
+from all build actions once at least one action
+completes. This parameter should only be used in
+special cases where a build tool performs a
+one-time generation of a side effect and then leaves
+it in place for the duration of the build. In this case,
+SCons will avoid executing these actions at the same time
+until the first one completes. After that, SCons will
+no longer treat this file as a side effect.
+</para>
 </summary>
 </scons_function>
 

SCons/EnvironmentTests.py

@@ -3298,13 +3298,15 @@ def test_SideEffect(self):
         env = self.TestEnvironment(LIB='lll', FOO='fff', BAR='bbb')
         env.File('mylll.pdb')
         env.Dir('mymmm.pdb')
+        env.File('mynnn.pdb')
 
         foo = env.Object('foo.obj', 'foo.cpp')[0]
         bar = env.Object('bar.obj', 'bar.cpp')[0]
         s = env.SideEffect('mylib.pdb', ['foo.obj', 'bar.obj'])[0]
         assert s.__class__.__name__ == 'Entry', s.__class__.__name__
         assert s.get_internal_path() == 'mylib.pdb'
         assert s.side_effect
+        assert not s.side_effect_temporary
         assert foo.side_effects == [s]
         assert bar.side_effects == [s]
 
@@ -3314,6 +3316,7 @@ def test_SideEffect(self):
         assert s.__class__.__name__ == 'File', s.__class__.__name__
         assert s.get_internal_path() == 'mylll.pdb'
         assert s.side_effect
+        assert not s.side_effect_temporary
         assert fff.side_effects == [s], fff.side_effects
         assert bbb.side_effects == [s], bbb.side_effects
 
@@ -3323,9 +3326,20 @@ def test_SideEffect(self):
         assert s.__class__.__name__ == 'Dir', s.__class__.__name__
         assert s.get_internal_path() == 'mymmm.pdb'
         assert s.side_effect
+        assert not s.side_effect_temporary
         assert ggg.side_effects == [s], ggg.side_effects
         assert ccc.side_effects == [s], ccc.side_effects
 
+        hhh = env.Object('hhh.obj', 'hhh.cpp')[0]
+        ddd = env.Object('ddd.obj', 'ddd.cpp')[0]
+        s = env.SideEffect('mynnn.pdb', ['hhh.obj', 'ddd.obj'], temporary=True)[0]
+        assert s.__class__.__name__ == 'File', s.__class__.__name__
+        assert s.get_internal_path() == 'mynnn.pdb'
+        assert s.side_effect
+        assert s.side_effect_temporary
+        assert hhh.side_effects == [s], hhh.side_effects
+        assert ddd.side_effects == [s], ddd.side_effects
+
     def test_Split(self):
         """Test the Split() method"""
         env = self.TestEnvironment(FOO = 'fff', BAR = 'bbb')

SCons/Node/FSTests.py

@@ -86,6 +86,9 @@ def Override(self, overrides):
     def _update(self, dict):
         pass
 
+    def get(self, key, default=None):
+        return default
+
 
 class Action:
     def __call__(self, targets, sources, env, **kw):

SCons/Node/__init__.py

@@ -42,6 +42,7 @@
 
 import collections
 import copy
+import os
 from itertools import chain
 
 try:
@@ -539,6 +540,7 @@ class Node(object, metaclass=NoSlotsPyPy):
                  'includes',
                  'attributes',
                  'side_effect',
+                 'side_effect_temporary',
                  'side_effects',
                  'linked',
                  '_memo',
@@ -602,6 +604,7 @@ def __init__(self):
         self.includes = None
         self.attributes = self.Attrs() # Generic place to stick information about the Node.
         self.side_effect = 0 # true iff this node is a side effect
+        self.side_effect_temporary = 0 # true iff this node is a temporary side effect
         self.side_effects = [] # the side effects of building this target
         self.linked = 0 # is this node linked to the variant directory?
         self.changed_since_last_build = 0
@@ -1059,6 +1062,9 @@ def add_to_implicit(self, deps):
             self._children_reset()
         self._add_child(self.implicit, self.implicit_set, deps)
 
+        if os.name == 'nt':
+            self._add_windows_typelib_dependencies(deps)
+
     def scan(self):
         """Scan this node's dependents for implicit dependencies."""
         # Don't bother scanning non-derived files, because we don't
@@ -1331,6 +1337,28 @@ def _add_child(self, collection, set, child):
         if added:
             self._children_reset()
 
+    def _add_windows_typelib_dependencies(self, deps):
+        """
+        Adds typelib side effects on Windows. This is useful if a file
+        #import's a type library because the compiler automatically generates
+        .tlh and .tli files in the build directory. The compiler does not
+        synchronize writes of these files so they should be considered side
+        effects. Make them temporary because once the first action runs,
+        no more writes will happen.
+        """
+        env = self.get_build_env()
+        if env.get('TYPELIB_SIDE_EFFECTS', True):
+            executor = self.get_executor()
+            targets = executor.get_all_targets() if executor else None
+            if targets:
+                for d in deps:
+                    if d.name.endswith(('.dll', '.tlb', '.ocx')):
+                        basename = os.path.splitext(d.name)[0]
+                        env.SideEffect([
+                            targets[0].dir.File(basename + '.tlh'),
+                            targets[0].dir.File(basename + '.tli'),
+                        ], targets, temporary=True)
+
     def set_specific_source(self, source):
         self.add_source(source)
         self._specific_sources = True

SCons/Taskmaster.py

@@ -452,6 +452,7 @@ def postprocess(self):
                 parents[p] = parents.get(p, 0) + 1
             t.waiting_parents = set()
 
+        temporary_side_effects_seen = set()
         for t in targets:
             if t.side_effects is not None:
                 for s in t.side_effects:
@@ -471,6 +472,18 @@ def postprocess(self):
                         if p.ref_count == 0:
                             self.tm.candidates.append(p)
 
+                    if s.side_effect_temporary:
+                        temporary_side_effects_seen.add(s)
+
+        # Remove all temporary side effects from all sources. This prevents
+        # problems if the temporary side effect is picked up again by another
+        # target.
+        for s in temporary_side_effects_seen:
+            for side_effect_source in s.sources:
+                side_effect_source.side_effects.remove(s)
+            s.sources = []
+
+
         for p, subtract in parents.items():
             p.ref_count = p.ref_count - subtract
             if T: T.write(self.trace_message('Task.postprocess()',

SCons/TaskmasterTests.py

@@ -59,6 +59,7 @@ def targets(self, node):
         self.waiting_parents = set()
         self.waiting_s_e = set()
         self.side_effect = 0
+        self.side_effect_temporary = 0
         self.side_effects = []
         self.alttargets = []
         self.postprocessed = None

test/Win32/typelib.py

@@ -0,0 +1,56 @@
+#!/usr/bin/env python
+#
+# __COPYRIGHT__
+#
+# Permission is hereby granted, free of charge, to any person obtaining
+# a copy of this software and associated documentation files (the
+# "Software"), to deal in the Software without restriction, including
+# without limitation the rights to use, copy, modify, merge, publish,
+# distribute, sublicense, and/or sell copies of the Software, and to
+# permit persons to whom the Software is furnished to do so, subject to
+# the following conditions:
+#
+# The above copyright notice and this permission notice shall be included
+# in all copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY
+# KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE
+# WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+# NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
+# LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
+# OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+# WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+#
+
+__revision__ = "__FILE__ __REVISION__ __DATE__ __DEVELOPER__"
+
+import sys
+import TestSCons
+
+# Run with 4 jobs to expose any race conditions related to the generated .tlh
+# and .tli files. Then run again with only 1 job to expose any issues handling
+# two different binaries one after another with the same typelib dependency.
+print_skip_message = True
+for arg in ['-j 4 .', '.']:
+    test = TestSCons.TestSCons()
+
+    if sys.platform != 'win32':
+        # Only print the skip message once.
+        msg = "Skipping typelib test on non-Windows platform '%s'\n" % sys.platform \
+            if print_skip_message else None
+        test.skip_test(msg)
+        print_skip_message = False
+
+    test.dir_fixture('typelib')
+
+    test.run('-j 4 .')
+    test.run('--clean .')
+    test.run()
+
+    test.pass_test()
+
+# Local Variables:
+# tab-width:4
+# indent-tabs-mode:nil
+# End:
+# vim: set expandtab tabstop=4 shiftwidth=4:

test/Win32/typelib/SConstruct

@@ -0,0 +1,15 @@
+import atexit
+
+program = Program('typelib.exe', source=[
+    'another.cpp', 'common.cpp', 'main.cpp'])
+
+program2 = Program('typelib2.exe', source=[
+    'another2.cpp', 'another3.cpp', 'common.cpp', 'main.cpp'])
+
+
+def VerifySideEffects():
+    # All side effects were temporary so they should be gone by now.
+    for source in program[0].sources:
+        assert 0 == len(source.side_effects), len(source.side_effects)
+
+atexit.register(VerifySideEffects)

test/Win32/typelib/another.cpp

@@ -0,0 +1 @@
+#include "common.h"

test/Win32/typelib/another2.cpp

@@ -0,0 +1 @@
+#include "common.h"

test/Win32/typelib/another3.cpp

@@ -0,0 +1 @@
+#include "common.h"

test/Win32/typelib/common.cpp

@@ -0,0 +1 @@
+#include "common.h"

test/Win32/typelib/common.h

@@ -0,0 +1 @@
+#import "test.tlb"

test/Win32/typelib/main.cpp

@@ -0,0 +1,6 @@
+#include "common.h"
+
+int main()
+{
+   return 0;
+}