Revert "merge revision(s) 62430c1: [Backport #21342]"

This reverts commit 4306c90.

Author: k0kubun

test/ruby/test_thread.rb

@@ -1557,97 +1557,4 @@ def test_pending_interrupt?
     assert_equal(true, t.pending_interrupt?(Exception))
     assert_equal(false, t.pending_interrupt?(ArgumentError))
   end
-
-  def test_deadlock_backtrace
-    bug21127 = '[ruby-core:120930] [Bug #21127]'
-
-    expected_stderr = [
-      /-:12:in 'Thread#join': No live threads left. Deadlock\? \(fatal\)\n/,
-      /2 threads, 2 sleeps current:\w+ main thread:\w+\n/,
-      /\* #<Thread:\w+ sleep_forever>\n/,
-      :*,
-      /^\s*-:6:in 'Object#frame_for_deadlock_test_2'/,
-      :*,
-      /\* #<Thread:\w+ -:10 sleep_forever>\n/,
-      :*,
-      /^\s*-:2:in 'Object#frame_for_deadlock_test_1'/,
-      :*,
-    ]
-
-    assert_in_out_err([], <<-INPUT, [], expected_stderr, bug21127)
-      def frame_for_deadlock_test_1
-        yield
-      end
-
-      def frame_for_deadlock_test_2
-        yield
-      end
-
-      q = Thread::Queue.new
-      t = Thread.new { frame_for_deadlock_test_1 { q.pop } }
-
-      frame_for_deadlock_test_2 { t.join }
-    INPUT
-  end
-
-  # [Bug #21342]
-  def test_unlock_locked_mutex_with_collected_fiber
-    bug21127 = '[ruby-core:120930] [Bug #21127]'
-    assert_separately([], "#{<<~"begin;"}\n#{<<~'end;'}")
-    begin;
-      5.times do
-        m = Mutex.new
-        Thread.new do
-          m.synchronize do
-          end
-        end.join
-        Fiber.new do
-          GC.start
-          m.lock
-        end.resume
-      end
-    end;
-  end
-
-  def test_unlock_locked_mutex_with_collected_fiber2
-    assert_separately([], "#{<<~"begin;"}\n#{<<~'end;'}")
-    begin;
-      MUTEXES = []
-      5.times do
-        m = Mutex.new
-        Fiber.new do
-          GC.start
-          m.lock
-        end.resume
-        MUTEXES << m
-      end
-      10.times do
-        MUTEXES.clear
-        GC.start
-      end
-    end;
-  end
-
-  def test_mutexes_locked_in_fiber_dont_have_aba_issue_with_new_fibers
-    assert_separately([], "#{<<~"begin;"}\n#{<<~'end;'}")
-    begin;
-      mutexes = 1000.times.map do
-        Mutex.new
-      end
-
-      mutexes.map do |m|
-        Fiber.new do
-          m.lock
-        end.resume
-      end
-
-      GC.start
-
-      1000.times.map do
-        Fiber.new do
-          raise "FAILED!" if mutexes.any?(&:owned?)
-        end.resume
-      end
-    end;
-  end
 end

thread.c

@@ -445,7 +445,7 @@ rb_threadptr_unlock_all_locking_mutexes(rb_thread_t *th)
         th->keeping_mutexes = mutex->next_mutex;
 
         // rb_warn("mutex #<%p> was not unlocked by thread #<%p>", (void *)mutex, (void*)th);
-        VM_ASSERT(mutex->fiber);
+
         const char *error_message = rb_mutex_unlock_th(mutex, th, mutex->fiber);
         if (error_message) rb_bug("invalid keeping_mutexes: %s", error_message);
     }

thread_sync.c

@@ -8,7 +8,6 @@ static VALUE rb_eClosedQueueError;
 /* Mutex */
 typedef struct rb_mutex_struct {
     rb_fiber_t *fiber;
-    VALUE thread; // even if the fiber is collected, we might need access to the thread in mutex_free
     struct rb_mutex_struct *next_mutex;
     struct ccan_list_head waitq; /* protected by GVL */
 } rb_mutex_t;
@@ -107,6 +106,8 @@ static const char* rb_mutex_unlock_th(rb_mutex_t *mutex, rb_thread_t *th, rb_fib
  *
  */
 
+#define mutex_mark ((void(*)(void*))0)
+
 static size_t
 rb_mutex_num_waiting(rb_mutex_t *mutex)
 {
@@ -122,39 +123,13 @@ rb_mutex_num_waiting(rb_mutex_t *mutex)
 
 rb_thread_t* rb_fiber_threadptr(const rb_fiber_t *fiber);
 
-static bool
-locked_p(rb_mutex_t *mutex)
-{
-    return mutex->fiber != 0;
-}
-
-static void
-mutex_mark(void *ptr)
-{
-    rb_mutex_t *mutex = ptr;
-    VALUE fiber;
-    if (locked_p(mutex)) {
-        fiber = rb_fiberptr_self(mutex->fiber); // rb_fiber_t* doesn't move along with fiber object
-        if (fiber) rb_gc_mark_movable(fiber);
-        rb_gc_mark_movable(mutex->thread);
-    }
-}
-
-static void
-mutex_compact(void *ptr)
-{
-    rb_mutex_t *mutex = ptr;
-    if (locked_p(mutex)) {
-        mutex->thread = rb_gc_location(mutex->thread);
-    }
-}
-
 static void
 mutex_free(void *ptr)
 {
     rb_mutex_t *mutex = ptr;
-    if (locked_p(mutex)) {
-        const char *err = rb_mutex_unlock_th(mutex, rb_thread_ptr(mutex->thread), mutex->fiber);
+    if (mutex->fiber) {
+        /* rb_warn("free locked mutex"); */
+        const char *err = rb_mutex_unlock_th(mutex, rb_fiber_threadptr(mutex->fiber), mutex->fiber);
         if (err) rb_bug("%s", err);
     }
     ruby_xfree(ptr);
@@ -168,7 +143,7 @@ mutex_memsize(const void *ptr)
 
 static const rb_data_type_t mutex_data_type = {
     "mutex",
-    {mutex_mark, mutex_free, mutex_memsize, mutex_compact,},
+    {mutex_mark, mutex_free, mutex_memsize,},
     0, 0, RUBY_TYPED_WB_PROTECTED | RUBY_TYPED_FREE_IMMEDIATELY
 };
 
@@ -229,13 +204,12 @@ rb_mutex_locked_p(VALUE self)
 {
     rb_mutex_t *mutex = mutex_ptr(self);
 
-    return RBOOL(locked_p(mutex));
+    return RBOOL(mutex->fiber);
 }
 
 static void
 thread_mutex_insert(rb_thread_t *thread, rb_mutex_t *mutex)
 {
-    RUBY_ASSERT(!mutex->next_mutex);
     if (thread->keeping_mutexes) {
         mutex->next_mutex = thread->keeping_mutexes;
     }
@@ -260,24 +234,10 @@ thread_mutex_remove(rb_thread_t *thread, rb_mutex_t *mutex)
 }
 
 static void
-mutex_set_owner(VALUE self, rb_thread_t *th, rb_fiber_t *fiber)
-{
-    rb_mutex_t *mutex = mutex_ptr(self);
-
-    mutex->thread = th->self;
-    mutex->fiber = fiber;
-    RB_OBJ_WRITTEN(self, Qundef, th->self);
-    if (fiber) {
-        RB_OBJ_WRITTEN(self, Qundef, rb_fiberptr_self(fiber));
-    }
-}
-
-static void
-mutex_locked(rb_thread_t *th, rb_fiber_t *fiber, VALUE self)
+mutex_locked(rb_thread_t *th, VALUE self)
 {
     rb_mutex_t *mutex = mutex_ptr(self);
 
-    mutex_set_owner(self, th, fiber);
     thread_mutex_insert(th, mutex);
 }
 
@@ -298,8 +258,9 @@ rb_mutex_trylock(VALUE self)
 
         rb_fiber_t *fiber = GET_EC()->fiber_ptr;
         rb_thread_t *th = GET_THREAD();
+        mutex->fiber = fiber;
 
-        mutex_locked(th, fiber, self);
+        mutex_locked(th, self);
         return Qtrue;
     }
     else {
@@ -367,7 +328,7 @@ do_mutex_lock(VALUE self, int interruptible_p)
                 rb_ensure(call_rb_fiber_scheduler_block, self, delete_from_waitq, (VALUE)&sync_waiter);
 
                 if (!mutex->fiber) {
-                    mutex_set_owner(self, th, fiber);
+                    mutex->fiber = fiber;
                 }
             }
             else {
@@ -397,7 +358,6 @@ do_mutex_lock(VALUE self, int interruptible_p)
                 rb_ractor_sleeper_threads_inc(th->ractor);
                 rb_check_deadlock(th->ractor);
 
-                RUBY_ASSERT(!th->locking_mutex);
                 th->locking_mutex = self;
 
                 ccan_list_add_tail(&mutex->waitq, &sync_waiter.node);
@@ -408,7 +368,7 @@ do_mutex_lock(VALUE self, int interruptible_p)
 
                 // unlocked by another thread while sleeping
                 if (!mutex->fiber) {
-                    mutex_set_owner(self, th, fiber);
+                    mutex->fiber = fiber;
                 }
 
                 rb_ractor_sleeper_threads_dec(th->ractor);
@@ -422,13 +382,10 @@ do_mutex_lock(VALUE self, int interruptible_p)
             if (interruptible_p) {
                 /* release mutex before checking for interrupts...as interrupt checking
                  * code might call rb_raise() */
-                if (mutex->fiber == fiber) {
-                    mutex->thread = Qfalse;
-                    mutex->fiber = NULL;
-                }
+                if (mutex->fiber == fiber) mutex->fiber = 0;
                 RUBY_VM_CHECK_INTS_BLOCKING(th->ec); /* may release mutex */
                 if (!mutex->fiber) {
-                    mutex_set_owner(self, th, fiber);
+                    mutex->fiber = fiber;
                 }
             }
             else {
@@ -447,7 +404,7 @@ do_mutex_lock(VALUE self, int interruptible_p)
         }
 
         if (saved_ints) th->ec->interrupt_flag = saved_ints;
-        if (mutex->fiber == fiber) mutex_locked(th, fiber, self);
+        if (mutex->fiber == fiber) mutex_locked(th, self);
     }
 
     RUBY_DEBUG_LOG("%p locked", mutex);