From 4ea8b31ec37dc1c11f1c80e558361951253f1c58 Mon Sep 17 00:00:00 2001
From: Boris Kolpackov <boris@codesynthesis.com>
Date: Sun, 6 Aug 2017 11:52:46 +0200
Subject: Only set variables on installed modules if we have lock

---
 build2/cc/pkgconfig.cxx | 56 ++++++++++++++++++++++++++++++-------------------
 1 file changed, 35 insertions(+), 21 deletions(-)

diff --git a/build2/cc/pkgconfig.cxx b/build2/cc/pkgconfig.cxx
index 542873b..4a5465d 100644
--- a/build2/cc/pkgconfig.cxx
+++ b/build2/cc/pkgconfig.cxx
@@ -556,31 +556,45 @@ namespace build2
           // information somehow (e.g., cxx_modules vs c_modules variable
           // names).
           //
-          target& mt (
-            targets.insert (*x_mod,
-                            mp.directory (),
-                            dir_path (),
-                            mf.base ().string (),
-                            mf.extension (),
-                            true, // Implied.
-                            trace).first);
-
-          //@@ TODO: if target already exists, then setting its variables is
-          //   not MT-safe. Perhaps use prerequisite-specific value?
+          auto tl (
+            targets.insert_locked (
+              *x_mod,
+              mp.directory (),
+              dir_path (),
+              mf.base ().string (),
+              mf.extension (),
+              true, // Implied.
+              trace));
+
+          target& mt (tl.first);
+
+          // If the target already exists, then setting its variables is not
+          // MT-safe. So currently we only do it if we have the lock (and thus
+          // nobody can see this target yet) assuming that this has already
+          // been done otherwise.
           //
-          mt.vars.assign (c_module_name) = move (mn);
-
-          // Set module properties. Note that if unspecified we should still
-          // set them to their default values since the hosting project may
-          // have them set to incompatible value.
+          // @@ This is not quite correct, though: this target could already
+          //    exist but for a "different purpose" (e.g., it could be used as
+          //    a header).
           //
+          if (tl.second.owns_lock ())
           {
-            value& v (mt.vars.assign (x_preprocessed)); // NULL
-            if (!pp.empty ()) v = move (pp);
-          }
+            mt.vars.assign (c_module_name) = move (mn);
 
-          {
-            mt.vars.assign (*x_symexport) = (se == "true");
+            // Set module properties. Note that if unspecified we should still
+            // set them to their default values since the hosting project may
+            // have them set to incompatible value.
+            //
+            {
+              value& v (mt.vars.assign (x_preprocessed)); // NULL
+              if (!pp.empty ()) v = move (pp);
+            }
+
+            {
+              mt.vars.assign (*x_symexport) = (se == "true");
+            }
+
+            tl.second.unlock ();
           }
 
           ps.push_back (prerequisite (mt));
-- 
cgit v1.1