From fd6445b7e8bab9d340be6f76688a8b96f1b52029 Mon Sep 17 00:00:00 2001 From: Koichi Sasada Date: Tue, 12 Nov 2019 10:02:47 +0900 Subject: [PATCH] Monitor#exit: check monitor ownership. Monitor#exit should be called by only onwer Thread. However, there is not check for it. --- ext/monitor/monitor.c | 27 ++++++++++++++++----------- test/monitor/test_monitor.rb | 23 +++++++++++++++++++++++ 2 files changed, 39 insertions(+), 11 deletions(-) diff --git a/ext/monitor/monitor.c b/ext/monitor/monitor.c index 2d399768b2..256fc4d7db 100644 --- a/ext/monitor/monitor.c +++ b/ext/monitor/monitor.c @@ -86,10 +86,25 @@ monitor_enter(VALUE monitor) } static VALUE -monitor_exit(VALUE monitor) +monitor_check_owner(VALUE monitor) { struct rb_monitor *mc = monitor_ptr(monitor); + if (!mc_owner_p(mc)) { + rb_raise(rb_eThreadError, "current thread not owner"); + } + return Qnil; +} + +static VALUE +monitor_exit(VALUE monitor) +{ + monitor_check_owner(monitor); + + struct rb_monitor *mc = monitor_ptr(monitor); + + if (mc->count <= 0) rb_bug("monitor_exit: count:%d\n", (int)mc->count); mc->count--; + if (mc->count == 0) { RB_OBJ_WRITE(monitor, &mc->owner, Qnil); rb_mutex_unlock(mc->mutex); @@ -111,16 +126,6 @@ monitor_owned_p(VALUE monitor) return (rb_mutex_locked_p(mc->mutex) && mc_owner_p(mc)) ? Qtrue : Qfalse; } -static VALUE -monitor_check_owner(VALUE monitor) -{ - struct rb_monitor *mc = monitor_ptr(monitor); - if (!mc_owner_p(mc)) { - rb_raise(rb_eThreadError, "current thread not owner"); - } - return Qnil; -} - static VALUE monitor_exit_for_cond(VALUE monitor) { diff --git a/test/monitor/test_monitor.rb b/test/monitor/test_monitor.rb index 49c34e067e..950db917e6 100644 --- a/test/monitor/test_monitor.rb +++ b/test/monitor/test_monitor.rb @@ -35,6 +35,29 @@ class TestMonitor < Test::Unit::TestCase assert_equal((1..10).to_a, ary) end + def test_exit + m = Monitor.new + m.enter + assert_equal true, m.mon_owned? + m.exit + assert_equal false, m.mon_owned? + + assert_raise ThreadError do + m.exit + end + + assert_equal false, m.mon_owned? + + m.enter + Thread.new{ + assert_raise(ThreadError) do + m.exit + end + }.join + assert_equal true, m.mon_owned? + m.exit + end + def test_enter_second_after_killed_thread th = Thread.start { @monitor.enter