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 <jwkozac...@gmail.com>
---
 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 osv-dev+unsubscr...@googlegroups.com.
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