The osv::run() API waits for the program it started to complete, so it
doesn't really need to start a new thread, and could run the program in
the existing thread.

And yet, it does create a new thread, because it uses the application::run()
API which starts a new application and returns to the caller immediately -
and therefore must create a new thread.

So this patch adds a new method application::run_and_join() which is like
an application::run() followed by an application::join() - except that
control does not return to the caller so we don't need a new thread.
osv::run() then uses this new application::run_and_join().

The osv_execve() suffered from the unnecessary thread in that the actual
application code does not run in the thread id returned by it, but rather
in a child thread. After this patch, it should return the right thread id.

Signed-off-by: Nadav Har'El <n...@scylladb.com>
---
 include/osv/app.hh | 21 +++++++++++++++++++++
 core/app.cc        | 29 +++++++++++++++++++++++++++++
 core/run.cc        |  3 +--
 3 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/include/osv/app.hh b/include/osv/app.hh
index 4fe261b..dbdf1da 100644
--- a/include/osv/app.hh
+++ b/include/osv/app.hh
@@ -114,6 +114,26 @@ public:
     int join();
 
     /**
+     * Start a new application and wait for it to terminate.
+     *
+     * run_and_join() is like run() followed by join(), with one important
+     * difference: because run() returns control to the caller, it needs
+     * to run the program in a new thread. But run_and_join() waits for
+     * the program to finish, so it can run it in the current thread,
+     * without creating a new one.
+     *
+     * \param command command to execute
+     * \param args Parameters which will be passed to program's main().
+     * \param new_program true if a new elf namespace must be started
+     * \param env pointer to an unordered_map than will be merged in current 
env
+     * \throw launch_error
+     */
+    static shared_app_t run_and_join(const std::string& command,
+            const std::vector<std::string>& args,
+            bool new_program = false,
+            const std::unordered_map<std::string, std::string> *env = nullptr);
+
+    /**
      * Installs a termination callback which will be called when
      * termination is requested or immediately if termination was
      * already requested.
@@ -168,6 +188,7 @@ private:
         return shared_from_this();
     }
     void start();
+    void start_and_join();
     void main();
     void run_main(std::string path, int argc, char** argv);
     void run_main();
diff --git a/core/app.cc b/core/app.cc
index ea00661..dc61413 100644
--- a/core/app.cc
+++ b/core/app.cc
@@ -135,6 +135,16 @@ void run(const std::vector<std::string>& args) {
     application::run(args);
 }
 
+shared_app_t application::run_and_join(const std::string& command,
+                      const std::vector<std::string>& args,
+                      bool new_program,
+                      const std::unordered_map<std::string, std::string> *env)
+{
+    auto app = std::make_shared<application>(command, args, new_program, env);
+    app->start_and_join();
+    return app;
+}
+
 application::application(const std::string& command,
                      const std::vector<std::string>& args,
                      bool new_program,
@@ -225,6 +235,25 @@ int application::join()
     return _return_code;
 }
 
+void application::start_and_join()
+{
+    // We start the new application code in the current thread. We temporarily
+    // change the app_runtime pointer of this thread, while keeping the old
+    // pointer saved and restoring it when the new application ends (keeping
+    // the shared pointer also keeps the calling application alive).
+    auto current_app = sched::thread::current()->app_runtime();
+    sched::thread::current()->set_app_runtime(runtime());
+    _thread = pthread_self(); // may be null if the caller is not a pthread.
+    main();
+    sched::thread::current()->set_app_runtime(current_app);
+    current_app.reset();
+    _joiner = sched::thread::current();
+    _runtime.reset();
+    sched::thread::wait_until([&] { return _terminated.load(); });
+    _termination_request_callbacks.clear();
+    _lib.reset();
+}
+
 TRACEPOINT(trace_app_main, "app=%p, cmd=%s", application*, const char*);
 TRACEPOINT(trace_app_main_ret, "return_code=%d", int);
 
diff --git a/core/run.cc b/core/run.cc
index 0184462..11631e1 100644
--- a/core/run.cc
+++ b/core/run.cc
@@ -8,8 +8,7 @@ std::shared_ptr<osv::application> run(std::string path,
                      bool new_program,
                      const std::unordered_map<std::string, std::string> *env)
 {
-    auto app = osv::application::run(path, args, new_program, env);
-    app->join();
+    auto app = osv::application::run_and_join(path, args, new_program, env);
     if (return_code) {
         *return_code = app->get_return_code();
     }
-- 
2.7.4

-- 
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to osv-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to