This patch replaces _parent_link/thread_list with a simple parent_thread_id
field to make walking parent-child list safer in elf::visible() method.

Signed-off-by: Waldemar Kozaczuk <[email protected]>
---
 core/elf.cc          | 27 +++++++++++++++++++--------
 core/sched.cc        |  8 +-------
 include/osv/sched.hh | 20 +++++---------------
 3 files changed, 25 insertions(+), 30 deletions(-)

diff --git a/core/elf.cc b/core/elf.cc
index a25fb3c1..86ccc5d9 100644
--- a/core/elf.cc
+++ b/core/elf.cc
@@ -151,17 +151,29 @@ bool object::visible(void) const
         return true;
     }
 
+    auto current_thread = sched::thread::current();
     if (level == VisibilityLevel::ThreadOnly) {
-        return thread == sched::thread::current();
+        return thread == current_thread;
     }
 
-    // Is current thread the same as "thread" or is it a child ?`
-    for (auto t = sched::thread::current(); t != nullptr; t = t->parent()) {
-        if (t == thread) {
-            return true;
-        }
+    // Is current thread the same as "thread" or is it a child of it?
+    if (thread == current_thread) {
+        return true;
     }
-    return false;
+
+    bool visible = false;
+    auto next_parent_id = current_thread->parent_id();
+    while (next_parent_id) {
+        sched::with_thread_by_id(next_parent_id, [&next_parent_id, &visible, 
thread] (sched::thread *t) {
+            if (t == thread) {
+                visible = true;
+                next_parent_id = 0;
+            } else {
+                next_parent_id = t ? t->parent_id() : 0;
+            }
+        });
+    }
+    return visible;
 }
 
 void object::set_visibility(elf::VisibilityLevel visibilityLevel)
@@ -171,7 +183,6 @@ void object::set_visibility(elf::VisibilityLevel 
visibilityLevel)
     _visibility_level.store(visibilityLevel, std::memory_order_release);
 }
 
-
 template <>
 void* object::lookup(const char* symbol)
 {
diff --git a/core/sched.cc b/core/sched.cc
index fde98432..06f849d1 100644
--- a/core/sched.cc
+++ b/core/sched.cc
@@ -1002,11 +1002,7 @@ thread::thread(std::function<void ()> func, attr attr, 
bool main, bool app)
                 sizeof(_attr._name) - 1);
     }
 
-    // Note that we never delete _parent_link. This is intentional as it lets 
us track
-    // parent-child relationship safely even when given thread is destroyed.
-    _parent_link = new thread_list();
-    _parent_link->_self.store(this);
-    _parent_link->_parent = (s_current && !main) ? s_current->_parent_link : 
nullptr;
+    _parent_id = s_current ? s_current->id() : 0;
 }
 
 static std::list<std::function<void ()>> exit_notifiers
@@ -1044,8 +1040,6 @@ thread::~thread()
 {
     cancel_this_thread_alarm();
 
-    _parent_link->_self.store(nullptr, std::memory_order_release);
-
     if (!_attr._detached) {
         join();
     }
diff --git a/include/osv/sched.hh b/include/osv/sched.hh
index 0c2aa143..0fb44f77 100644
--- a/include/osv/sched.hh
+++ b/include/osv/sched.hh
@@ -364,13 +364,6 @@ public:
             return *this;
         }
     };
-    // Simple single-linked list that allows to keep track of
-    // "parent-child" relationship between threads. The "parent" is
-    // the thread that constructs given "child" thread
-    struct thread_list {
-        std::atomic<thread*> _self;
-        thread_list *_parent;
-    };
 
 private:
     // Unlike the detached user visible attribute, those are internal to
@@ -640,7 +633,10 @@ public:
     {
         return static_cast<T*>(do_remote_thread_local_var(var));
     }
-    thread *parent();
+    unsigned int parent_id() const
+    {
+        return _parent_id;
+    }
 private:
     virtual void timer_fired() override;
     struct detached_state;
@@ -777,7 +773,7 @@ private:
     inline void cputime_estimator_get(
             osv::clock::uptime::time_point &running_since,
             osv::clock::uptime::duration &total_cpu_time);
-    thread_list *_parent_link;
+    unsigned int _parent_id;
 };
 
 class thread_handle {
@@ -1304,12 +1300,6 @@ inline thread_handle thread::handle()
     return thread_handle(*this);
 }
 
-inline thread* thread::parent()
-{
-    auto _parent = _parent_link->_parent;
-    return _parent ? _parent->_self.load(std::memory_order_acquire) : nullptr;
-}
-
 inline void* thread::get_tls(ulong module)
 {
     if (module >= _tls.size()) {
-- 
2.20.1

-- 
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 [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/osv-dev/20200112224434.13489-1-jwkozaczuk%40gmail.com.

Reply via email to