Fix bug in create_new_target_locked()

8 files changed, 49 insertions, 10 deletions

diff --git a/libbuild2/algorithm.cxx b/libbuild2/algorithm.cxx
index f859eef..39df018 100644
--- a/libbuild2/algorithm.cxx
+++ b/libbuild2/algorithm.cxx
@@ -312,7 +312,7 @@ namespace build2
                                    target_decl::implied,
                                    trace));
 
-    assert (r.second.owns_lock ());
+    assert (r.second);
 
     target& m (r.first);
     *mp = &m;
diff --git a/libbuild2/bash/rule.cxx b/libbuild2/bash/rule.cxx
index 9fc89d7..8b47fb0 100644
--- a/libbuild2/bash/rule.cxx
+++ b/libbuild2/bash/rule.cxx
@@ -168,7 +168,7 @@ namespace build2
 
                   // Only set path/mtime on first insertion.
                   //
-                  if (rp.second.owns_lock ())
+                  if (rp.second)
                     pt.path_mtime (move (ap), mt);
 
                   // Save the length of the import path in auxuliary data. We
diff --git a/libbuild2/cc/common.txx b/libbuild2/cc/common.txx
index ce922cc..d14f966 100644
--- a/libbuild2/cc/common.txx
+++ b/libbuild2/cc/common.txx
@@ -27,7 +27,7 @@ namespace build2
                                          target_decl::implied,
                                          trace));
 
-      assert (!exist || !p.second.owns_lock ());
+      assert (!exist || !p.second);
       r = &p.first.template as<T> ();
       return move (p.second);
     }
diff --git a/libbuild2/cc/compile-rule.cxx b/libbuild2/cc/compile-rule.cxx
index a11b7ab..b96c39d 100644
--- a/libbuild2/cc/compile-rule.cxx
+++ b/libbuild2/cc/compile-rule.cxx
@@ -6051,7 +6051,7 @@ namespace build2
       // Note that this is racy and someone might have created this target
       // while we were preparing the prerequisite list.
       //
-      if (p.second.owns_lock ())
+      if (p.second)
       {
         bt.prerequisites (move (ps));
 
@@ -6129,7 +6129,7 @@ namespace build2
       // Note that this is racy and someone might have created this target
       // while we were preparing the prerequisite list.
       //
-      if (p.second.owns_lock ())
+      if (p.second)
         bt.prerequisites (move (ps));
 
       return bt;
diff --git a/libbuild2/file.cxx b/libbuild2/file.cxx
index def49b8..219a5f1 100644
--- a/libbuild2/file.cxx
+++ b/libbuild2/file.cxx
@@ -2682,7 +2682,7 @@ namespace build2
         // a simple name via PATH search and as an absolute path in which case
         // the first import will determine the path).
         //
-        if (r.second.owns_lock ())
+        if (r.second)
         {
           r.first.as<exe> ().process_path (move (pp));
           r.second.unlock ();
diff --git a/libbuild2/search.cxx b/libbuild2/search.cxx
index b341c85..25a4199 100644
--- a/libbuild2/search.cxx
+++ b/libbuild2/search.cxx
@@ -278,9 +278,16 @@ namespace build2
                                        tk.ext,
                                        target_decl::prereq_new,
                                        trace));
+    l5 ([&]
+        {
+          diag_record dr (trace);
+          if (r.second)
+            dr << "new target " << r.first.key_locked ();
+          else
+            dr << "existing target " << r.first;
+          dr << " for prerequisite " << pk;
+        });
 
-    l5 ([&]{trace << (r.second ? "new" : "existing") << " target " << r.first
-                  << " for prerequisite " << pk;});
     return r;
   }
 }
diff --git a/libbuild2/target.hxx b/libbuild2/target.hxx
index e1d91e4..8c7ccee 100644
--- a/libbuild2/target.hxx
+++ b/libbuild2/target.hxx
@@ -147,6 +147,10 @@ namespace build2
     const string* ext () const; // Return NULL if not specified.
     const string& ext (string);
 
+    // As above but assume targets mutex is locked.
+    //
+    const string* ext_locked () const;
+
     const dir_path&
     out_dir () const {return out.empty () ? dir : out;}
 
@@ -284,6 +288,11 @@ namespace build2
     target_key
     key () const;
 
+    // As above but assume targets mutex is locked.
+    //
+    target_key
+    key_locked () const;
+
     names
     as_name () const;
 
@@ -811,6 +820,11 @@ namespace build2
   inline bool
   operator!= (const target& x, const target& y) {return !(x == y);}
 
+  // Note that if the targets mutex is locked, then calling this operator
+  // will lead to a deadlock. Instead, do:
+  //
+  // ... << t.key_locked () << ...
+  //
   ostream&
   operator<< (ostream&, const target&);
 
@@ -1385,7 +1399,7 @@ namespace build2
                              decl,
                              t));
 
-      return pair<target&, bool> (p.first, p.second.owns_lock ());
+      return pair<target&, bool> (p.first, p.second);
     }
 
     // Note that the following versions always enter implied targets.
diff --git a/libbuild2/target.ixx b/libbuild2/target.ixx
index df1cf5b..2d1906e 100644
--- a/libbuild2/target.ixx
+++ b/libbuild2/target.ixx
@@ -12,10 +12,16 @@ namespace build2
   // target
   //
   inline const string* target::
+  ext_locked () const
+  {
+    return *ext_ ? &**ext_ : nullptr;
+  }
+
+  inline const string* target::
   ext () const
   {
     slock l (ctx.targets.mutex_);
-    return *ext_ ? &**ext_ : nullptr;
+    return ext_locked ();
   }
 
   inline target_key target::
@@ -30,6 +36,18 @@ namespace build2
       e != nullptr ? optional<string> (*e) : nullopt};
   }
 
+  inline target_key target::
+  key_locked () const
+  {
+    const string* e (ext_locked ());
+    return target_key {
+      &type (),
+      &dir,
+      &out,
+      &name,
+      e != nullptr ? optional<string> (*e) : nullopt};
+  }
+
   inline names target::
   as_name () const
   {