Reimplement testscript builtins without thread detach, future/promise

4 files changed, 62 insertions, 54 deletions

diff --git a/build2/test/script/builtin b/build2/test/script/builtin
index 226a0e0..7d902ea 100644
--- a/build2/test/script/builtin
+++ b/build2/test/script/builtin
@@ -18,26 +18,48 @@ namespace build2
     {
       class scope;
 
+      // A process/thread-like object representing a running builtin.
+      //
+      // For now, instead of allocating the result storage dynamically, we
+      // expect it to be provided by the caller.
+      //
+      class builtin
+      {
+      public:
+        uint8_t
+        wait () {if (t_.joinable ()) t_.join (); return r_;}
+
+        ~builtin () {wait ();}
+
+      public:
+        builtin (uint8_t& r, thread&& t = thread ()): r_ (r), t_ (move (t)) {}
+
+        builtin (builtin&&) = default;
+        builtin& operator= (builtin&&) = default;
+
+      private:
+        uint8_t& r_;
+        thread t_;
+      };
+
       // Start builtin command. Throw system_error on failure.
       //
       // Note that unlike argc/argv, our args don't include the program name.
       //
-      // Also note that the future object being returned doesn't block in dtor
-      // until the builtin command terminates.
-      //
-      using builtin = future<uint8_t> (scope&,
-                                       const strings& args,
-                                       auto_fd in, auto_fd out, auto_fd err);
+      using builtin_func = builtin (scope&,
+                                    uint8_t& result,
+                                    const strings& args,
+                                    auto_fd in, auto_fd out, auto_fd err);
 
-      class builtin_map: public std::map<string, builtin*>
+      class builtin_map: public std::map<string, builtin_func*>
       {
       public:
-        using base = std::map<string, builtin*>;
+        using base = std::map<string, builtin_func*>;
         using base::base;
 
         // Return NULL if not a builtin.
         //
-        builtin*
+        builtin_func*
         find (const string& n) const
         {
           auto i (base::find (n));
diff --git a/build2/test/script/builtin.cxx b/build2/test/script/builtin.cxx
index 3305c3a..d96058e 100644
--- a/build2/test/script/builtin.cxx
+++ b/build2/test/script/builtin.cxx
@@ -11,7 +11,6 @@
 #endif
 
 #include <locale>
-#include <thread>
 #include <ostream>
 #include <sstream>
 
@@ -43,14 +42,6 @@ namespace build2
       using builtin_impl = uint8_t (scope&,
                                     const strings& args,
                                     auto_fd in, auto_fd out, auto_fd err);
-      static future<uint8_t>
-      to_future (uint8_t status)
-      {
-        promise<uint8_t> p;
-        future<uint8_t> f (p.get_future ());
-        p.set_value (status);
-        return f;
-      }
 
       // Operation failed, diagnostics has already been issued.
       //
@@ -521,10 +512,10 @@ namespace build2
       //
       // Note: can be executed synchronously.
       //
-      static future<uint8_t>
-      false_ (scope&, const strings&, auto_fd, auto_fd, auto_fd)
+      static builtin
+      false_ (scope&, uint8_t& r, const strings&, auto_fd, auto_fd, auto_fd)
       {
-        return to_future (1);
+        return builtin (r = 1);
       }
 
       // true
@@ -533,10 +524,10 @@ namespace build2
       //
       // Note: can be executed synchronously.
       //
-      static future<uint8_t>
-      true_ (scope&, const strings&, auto_fd, auto_fd, auto_fd)
+      static builtin
+      true_ (scope&, uint8_t& r, const strings&, auto_fd, auto_fd, auto_fd)
       {
-        return to_future (0);
+        return builtin (r = 0);
       }
 
       // Create a directory if not exist and its parent directories if
@@ -1362,15 +1353,11 @@ namespace build2
                     scope& sp,
                     const strings& args,
                     auto_fd in, auto_fd out, auto_fd err,
-                    promise<uint8_t> p) noexcept
+                    uint8_t& r) noexcept
       {
         try
         {
-          // The use of set_value_at_thread_exit() would be more appropriate
-          // but the function is not supported by old versions of g++ (e.g.,
-          // not in 4.9). There could also be overhead associated with it.
-          //
-          p.set_value (fn (sp, args, move (in), move (out), move (err)));
+          r = fn (sp, args, move (in), move (out), move (err));
         }
         catch (const std::exception& e)
         {
@@ -1382,44 +1369,43 @@ namespace build2
 
       // Run builtin implementation asynchronously.
       //
-      static future<uint8_t>
+      static builtin
       async_impl (builtin_impl fn,
                   scope& sp,
+                  uint8_t& r,
                   const strings& args,
                   auto_fd in, auto_fd out, auto_fd err)
       {
-        promise<uint8_t> p;
-        future<uint8_t> f (p.get_future ());
-
-        thread t (thread_thunk,
-                  fn,
-                  ref (sp),
-                  cref (args),
-                  move (in), move (out), move (err),
-                  move (p));
-
-        t.detach ();
-        return f;
+        return builtin (r,
+                        thread (thread_thunk,
+                                fn,
+                                ref (sp),
+                                cref (args),
+                                move (in), move (out), move (err),
+                                ref (r)));
       }
 
       template <builtin_impl fn>
-      static future<uint8_t>
+      static builtin
       async_impl (scope& sp,
+                  uint8_t& r,
                   const strings& args,
                   auto_fd in, auto_fd out, auto_fd err)
       {
-        return async_impl (fn, sp, args, move (in), move (out), move (err));
+        return async_impl (fn, sp, r, args, move (in), move (out), move (err));
       }
 
       // Run builtin implementation synchronously.
       //
       template <builtin_impl fn>
-      static future<uint8_t>
+      static builtin
       sync_impl (scope& sp,
+                 uint8_t& r,
                  const strings& args,
                  auto_fd in, auto_fd out, auto_fd err)
       {
-        return to_future (fn (sp, args, move (in), move (out), move (err)));
+        r = fn (sp, args, move (in), move (out), move (err));
+        return builtin (r, thread ());
       }
 
       const builtin_map builtins
diff --git a/build2/test/script/runner.cxx b/build2/test/script/runner.cxx
index 3c5f8ba..e969d5a 100644
--- a/build2/test/script/runner.cxx
+++ b/build2/test/script/runner.cxx
@@ -1424,7 +1424,7 @@ namespace build2
         assert (ofd.get () != -1 && efd.get () != -1);
 
         optional<process_exit> exit;
-        builtin* b (builtins.find (c.program.string ()));
+        builtin_func* bf (builtins.find (c.program.string ()));
 
         bool success;
 
@@ -1439,7 +1439,7 @@ namespace build2
           return args;
         };
 
-        if (b != nullptr)
+        if (bf != nullptr)
         {
           // Execute the builtin.
           //
@@ -1448,12 +1448,13 @@ namespace build2
 
           try
           {
-            future<uint8_t> f (
-              (*b) (sp, c.arguments, move (ifd), move (ofd), move (efd)));
+            uint8_t r; // Storage.
+            builtin b (
+              bf (sp, r, c.arguments, move (ifd), move (ofd), move (efd)));
 
             success = run_pipe (sp, nc, ec, move (p.in), ci + 1, li, ll, diag);
 
-            exit = process_exit (f.get ());
+            exit = process_exit (b.wait ());
           }
           catch (const system_error& e)
           {
diff --git a/build2/types b/build2/types
index de0ae3f..9a5be75 100644
--- a/build2/types
+++ b/build2/types
@@ -20,7 +20,6 @@
 
 #include <mutex>
 #include <atomic>
-#include <future>
 #include <thread>
 #include <condition_variable>
 
@@ -128,7 +127,7 @@ namespace build2
   using std::defer_lock;
   using std::adopt_lock;
 
-  using std::future;
+  using std::thread;
   namespace this_thread = std::this_thread;
 
   // Exceptions.