Removed SYNC_TL and other threading fixes.

Really not clear on how SYNC_TL got into Rubinius. If every thread has it's own mutex, there's no mutual exclusion.
rubinius · Jun 16, 2015 · 4b5b02d · 4b5b02d
1 parent d3bc182
commit 4b5b02d
Show file tree

Hide file tree

Showing 21 changed files with 129 additions and 106 deletions.
diff --git a/vm/builtin/thread.cpp b/vm/builtin/thread.cpp
@@ -269,17 +269,10 @@ namespace rubinius {
     SharedState& shared = vm->shared;
     State state_obj(vm), *state = &state_obj;
 
-    std::string thread_name;
+    vm->set_current_thread();
 
-    {
-      std::ostringstream tn;
-      tn << "rbx.ruby." << vm->thread_id();
-      VM::set_current(vm, tn.str());
-      thread_name = tn.str();
-    }
-
-    RUBINIUS_THREAD_START(const_cast<RBX_DTRACE_CHAR_P>(thread_name.c_str()),
-                          vm->thread_id(), 0);
+    RUBINIUS_THREAD_START(
+        const_cast<RBX_DTRACE_CHAR_P>(vm->name().c_str()), vm->thread_id(), 0);
 
     if(cDebugThreading) {
       utilities::logger::debug("Thread: start thread: id: %d, pthread: %d",
@@ -341,8 +334,8 @@ namespace rubinius {
 
     vm->set_zombie();
 
-    RUBINIUS_THREAD_STOP(const_cast<RBX_DTRACE_CHAR_P>(thread_name.c_str()),
-                         vm->thread_id(), 0);
+    RUBINIUS_THREAD_STOP(
+        const_cast<RBX_DTRACE_CHAR_P>(vm->name().c_str()), vm->thread_id(), 0);
 
     return 0;
   }

diff --git a/vm/console.hpp b/vm/console.hpp
@@ -1,7 +1,6 @@
 #ifndef RBX_CONSOLE_HPP
 #define RBX_CONSOLE_HPP
 
-#include "lock.hpp"
 #include "internal_threads.hpp"
 
 #include "gc/root.hpp"
@@ -66,7 +65,7 @@ namespace rubinius {
       Class* server_class(STATE);
     };
 
-    class Listener : public InternalThread, public Lockable {
+    class Listener : public InternalThread {
       Console* console_;
 
       TypedRoot<FSEvent*> fsevent_;
@@ -88,7 +87,7 @@ namespace rubinius {
 
     typedef std::list<char*> RequestList;
 
-    class Response : public InternalThread, public Lockable {
+    class Response : public InternalThread {
       Console* console_;
 
       TypedRoot<Channel*> inbox_;
@@ -122,7 +121,7 @@ namespace rubinius {
       void write_response(STATE, const char* response, native_int size);
     };
 
-    class Request : public InternalThread, public Lockable {
+    class Request : public InternalThread {
       Console* console_;
       Response* response_;
 

diff --git a/vm/environment.cpp b/vm/environment.cpp
@@ -104,7 +104,7 @@ namespace rubinius {
 
     check_io_descriptors();
 
-    root_vm = shared->new_vm();
+    root_vm = shared->new_vm("rbx.ruby.main");
     root_vm->metrics().init(metrics::eRubyMetrics);
     state = new State(root_vm);
 

diff --git a/vm/gc/finalize.hpp b/vm/gc/finalize.hpp
@@ -1,7 +1,6 @@
 #ifndef RBX_GC_FINALIZE_HPP
 #define RBX_GC_FINALIZE_HPP
 
-#include "lock.hpp"
 #include "internal_threads.hpp"
 
 #include "gc/finalize.hpp"
@@ -59,7 +58,7 @@ namespace rubinius {
   typedef std::list<FinalizeObject> FinalizeObjects;
   typedef std::list<FinalizeObjects*> FinalizeObjectsList;
 
-  class FinalizerThread : public InternalThread, public Lockable {
+  class FinalizerThread : public InternalThread {
   public:
     class iterator {
       FinalizerThread* handler_;

diff --git a/vm/gc/immix_marker.hpp b/vm/gc/immix_marker.hpp
@@ -1,7 +1,6 @@
 #ifndef RBX_GC_IMMIX_MARKER_HPP
 #define RBX_GC_IMMIX_MARKER_HPP
 
-#include "lock.hpp"
 #include "internal_threads.hpp"
 
 #include "gc/root.hpp"
@@ -13,7 +12,7 @@ namespace rubinius {
   class ImmixGC;
   class GCData;
 
-  class ImmixMarker : public InternalThread, public Lockable {
+  class ImmixMarker : public InternalThread {
     ImmixGC* immix_;
     GCData* data_;
 

diff --git a/vm/gc/managed.cpp b/vm/gc/managed.cpp
@@ -5,16 +5,28 @@
 #include "shared_state.hpp"
 #include "metrics.hpp"
 
+#include <sstream>
+
 namespace rubinius {
   utilities::thread::ThreadData<ManagedThread*> _current_thread;
 
-  ManagedThread::ManagedThread(uint32_t id, SharedState& ss, ManagedThread::Kind kind)
-    : shared_(ss)
-    , name_(kind == eRuby ? "<ruby>" : "<system>")
+  ManagedThread::ManagedThread(uint32_t id, SharedState& ss,
+      ManagedThread::Kind kind, const char* name)
+    : Lockable(true)
+    , shared_(ss)
     , run_state_(eIndependent)
     , kind_(kind)
+    , os_thread_(pthread_self())
     , id_(id)
   {
+    if(name) {
+      name_ = std::string(name);
+    } else {
+      std::ostringstream thread_name;
+      thread_name << "rbx.ruby." << id_;
+      name_ = thread_name.str();
+    }
+
     metrics_.init(metrics::eNone);
   }
 
@@ -28,9 +40,9 @@ namespace rubinius {
     return _current_thread.get();
   }
 
-  void ManagedThread::set_current(ManagedThread* th, std::string name) {
+  void ManagedThread::set_current_thread(ManagedThread* th) {
+    utilities::thread::Thread::set_os_name(th->name().c_str());
     th->os_thread_ = pthread_self();
-    th->set_name(name);
     _current_thread.set(th);
   }
 }
diff --git a/vm/gc/managed.hpp b/vm/gc/managed.hpp
@@ -10,6 +10,7 @@
 
 #include <algorithm>
 #include <vector>
+#include <string>
 
 namespace rubinius {
   class SharedState;
@@ -24,7 +25,8 @@ namespace rubinius {
     friend class WorldState;
 
     enum Kind {
-      eRuby, eSystem
+      eRuby,
+      eSystem
     };
 
     enum RunState {
@@ -51,9 +53,12 @@ namespace rubinius {
     uint32_t id_;
 
   public:
-    ManagedThread(uint32_t id, SharedState& ss, Kind kind);
+    ManagedThread(uint32_t id, SharedState& ss, Kind kind, const char* name);
     ~ManagedThread();
 
+    static void set_current_thread(ManagedThread* vm);
+    static ManagedThread* current();
+
     Roots& roots() {
       return roots_;
     }
@@ -99,19 +104,13 @@ namespace rubinius {
     }
 
     VM* as_vm() {
-      if(kind_ == eRuby) return reinterpret_cast<VM*>(this);
-      return 0;
+      return reinterpret_cast<VM*>(this);
     }
 
     std::string name() const {
       return name_;
     }
 
-    void set_name(std::string name) {
-      name_ = name;
-      utilities::thread::Thread::set_os_name(name.c_str());
-    }
-
     uint32_t thread_id() const {
       return id_;
     }
@@ -131,10 +130,6 @@ namespace rubinius {
     metrics::MetricsData& metrics() {
       return metrics_;
     }
-
-  public:
-    static ManagedThread* current();
-    static void set_current(ManagedThread* vm, std::string name);
   };
 }
 

diff --git a/vm/global_cache.hpp b/vm/global_cache.hpp
@@ -44,7 +44,9 @@ namespace rubinius {
     static bool resolve(STATE, Symbol* name, Dispatch& msg, LookupData& lookup);
     bool resolve_i(STATE, Symbol* name, Dispatch& msg, LookupData& lookup);
 
-    GlobalCache() {
+    GlobalCache()
+      : Lockable(true)
+    {
       reset();
     }
 

diff --git a/vm/internal_threads.cpp b/vm/internal_threads.cpp
@@ -12,8 +12,7 @@ namespace rubinius {
   using namespace utilities;
 
   InternalThread::InternalThread(STATE, std::string name, StackSize stack_size)
-    : vm_(state->shared().new_vm())
-    , name_(name)
+    : vm_(state->shared().new_vm(name.c_str()))
     , thread_running_(false)
     , stack_size_(stack_size)
     , metrics_(vm_->metrics())
@@ -29,14 +28,11 @@ namespace rubinius {
     SharedState& shared = vm->shared;
     State state_obj(vm), *state = &state_obj;
 
-    state->vm()->set_run_state(ManagedThread::eIndependent);
+    vm->set_current_thread();
+    vm->set_run_state(ManagedThread::eIndependent);
 
-    RBX_DTRACE_CHAR_P thread_name =
-      const_cast<RBX_DTRACE_CHAR_P>(thread->name_.c_str());
-    vm->set_name(thread_name);
-
-    RUBINIUS_THREAD_START(const_cast<RBX_DTRACE_CHAR_P>(thread_name),
-                          vm->thread_id(), 1);
+    RUBINIUS_THREAD_START(
+        const_cast<RBX_DTRACE_CHAR_P>(vm->name().c_str()), vm->thread_id(), 1);
 
     NativeMethod::init_thread(state);
 
@@ -48,12 +44,10 @@ namespace rubinius {
 
     NativeMethod::cleanup_thread(state);
 
-    RUBINIUS_THREAD_STOP(const_cast<RBX_DTRACE_CHAR_P>(thread_name),
-                         vm->thread_id(), 1);
+    RUBINIUS_THREAD_STOP(
+        const_cast<RBX_DTRACE_CHAR_P>(vm->name().c_str()), vm->thread_id(), 1);
 
-    if(state->vm()->run_state() != ManagedThread::eIndependent) {
-      shared.gc_independent();
-    }
+    shared.gc_independent();
 
     return 0;
   }
@@ -75,7 +69,7 @@ namespace rubinius {
 
     if(int error = pthread_create(&vm_->os_thread(), &attrs,
           InternalThread::run, (void*)this)) {
-      logger::fatal("%s: %s: create thread failed", strerror(error), name_.c_str());
+      logger::fatal("%s: %s: create thread failed", strerror(error), vm_->name().c_str());
       ::abort();
     }
 

diff --git a/vm/internal_threads.hpp b/vm/internal_threads.hpp
@@ -1,6 +1,8 @@
 #ifndef RBX_AUXILIARY_THREADS_H
 #define RBX_AUXILIARY_THREADS_H
 
+#include "lock.hpp"
+
 #include "util/thread.hpp"
 
 #include <string>
@@ -15,7 +17,6 @@ namespace rubinius {
 
   class InternalThread {
     VM* vm_;
-    std::string name_;
     bool thread_running_;
     uint32_t stack_size_;
 

diff --git a/vm/llvm/state.hpp b/vm/llvm/state.hpp
@@ -42,7 +42,6 @@
 #include "gc/managed.hpp"
 #include "internal_threads.hpp"
 #include "configuration.hpp"
-#include "lock.hpp"
 #include "metrics.hpp"
 #include "util/thread.hpp"
 
@@ -71,7 +70,7 @@ namespace rubinius {
     cMachineCode = 4
   };
 
-  class LLVMState : public InternalThread, public Lockable {
+  class LLVMState : public InternalThread {
     jit::RubiniusJITMemoryManager* memory_;
     llvm::JITEventListener* jit_event_listener_;
 

diff --git a/vm/lock.hpp b/vm/lock.hpp
@@ -51,6 +51,10 @@ namespace rubinius {
 
   class Mutex : public Lock, public utilities::thread::Mutex {
   public:
+    Mutex(bool recursive)
+      : utilities::thread::Mutex(recursive)
+    { }
+
     bool mutex_p() {
       return true;
     }
@@ -153,6 +157,10 @@ namespace rubinius {
     Mutex mutex_;
 
   public:
+    Lockable(bool recursive)
+      : mutex_(recursive)
+    { }
+
     Mutex& mutex() {
       return mutex_;
     }
@@ -225,8 +233,6 @@ namespace rubinius {
 
 
 #define SYNC(__state) LockableScopedLock __lsl_guard__(__state, this, __FILE__, __LINE__)
-#define SYNC_TL LockableScopedLock __lsl_guard__(ManagedThread::current(), this, __FILE__, __LINE__)
-
 #define UNSYNC __lsl_guard__.unlock()
 #define RESYNC __lsl_guard__.relock()
 }

diff --git a/vm/metrics.hpp b/vm/metrics.hpp
@@ -1,7 +1,6 @@
 #ifndef RBX_METRICS_HPP
 #define RBX_METRICS_HPP
 
-#include "lock.hpp"
 #include "internal_threads.hpp"
 
 #include "gc/root.hpp"
@@ -375,7 +374,7 @@ namespace rubinius {
       void reinit();
     };
 
-    class Metrics : public InternalThread, public Lockable {
+    class Metrics : public InternalThread {
       bool enabled_;
 
       TypedRoot<Tuple*> values_;