From 23444302d9200bcc41ce279a529f73cad63c3f05 Mon Sep 17 00:00:00 2001 From: normal Date: Fri, 4 Jan 2019 13:14:11 +0000 Subject: [PATCH] introduce rb_nogvl C-API to mark ubf as async-signal-safe zlib and bignum both contain unblocking functions which are async-signal-safe and do not require spawning additional threads. We can execute those functions directly in signal handlers without incurring overhead of extra threads, so provide C-API users the ability to deal with that. Other C-API users may have similar need. This flexible API can supercede existing uses of rb_thread_call_without_gvl and rb_thread_call_without_gvl2 by introducing a flags argument to control behavior. Note: this API is NOT finalized. It needs approval from other committers. I prefer shorter name than previous rb_thread_call_without_gvl* functions because my eyes requires big fonts. [Bug #15499] git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@66712 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- bignum.c | 3 +- .../gvl/call_without_gvl/call_without_gvl.c | 41 +++++++++++++++++++ ext/zlib/zlib.c | 6 ++- include/ruby/thread.h | 12 ++++++ test/-ext-/gvl/test_ubf_async_safe.rb | 20 +++++++++ thread.c | 24 +++++++---- thread_pthread.c | 5 +++ vm_core.h | 3 ++ 8 files changed, 103 insertions(+), 11 deletions(-) create mode 100644 test/-ext-/gvl/test_ubf_async_safe.rb diff --git a/bignum.c b/bignum.c index ee3b49fd04..5285e5979e 100644 --- a/bignum.c +++ b/bignum.c @@ -2572,6 +2572,7 @@ bigdivrem1(void *ptr) return 0; } +/* async-signal-safe */ static void rb_big_stop(void *ptr) { @@ -2636,7 +2637,7 @@ bigdivrem_restoring(BDIGIT *zds, size_t zn, BDIGIT *yds, size_t yn) if (bds.zn > 10000 || bds.yn > 10000) { retry: bds.stop = Qfalse; - rb_thread_call_without_gvl(bigdivrem1, &bds, rb_big_stop, &bds); + rb_nogvl(bigdivrem1, &bds, rb_big_stop, &bds, RB_NOGVL_UBF_ASYNC_SAFE); if (bds.stop == Qtrue) { /* execute trap handler, but exception was not raised. */ diff --git a/ext/-test-/gvl/call_without_gvl/call_without_gvl.c b/ext/-test-/gvl/call_without_gvl/call_without_gvl.c index f3071d5768..654c979479 100644 --- a/ext/-test-/gvl/call_without_gvl/call_without_gvl.c +++ b/ext/-test-/gvl/call_without_gvl/call_without_gvl.c @@ -27,8 +27,49 @@ thread_runnable_sleep(VALUE thread, VALUE timeout) return thread; } +struct loop_ctl { + int notify_fd; + volatile int stop; +}; + +static void * +do_loop(void *p) +{ + struct loop_ctl *ctl = p; + + /* tell the waiting process they can interrupt us, now */ + write(ctl->notify_fd, "", 1); + + while (!ctl->stop) { + struct timeval tv = { 0, 10000 }; + select(0, NULL, NULL, NULL, &tv); + } + return 0; +} + +static void +stop_set(void *p) +{ + struct loop_ctl *ctl = p; + + ctl->stop = 1; +} + +static VALUE +thread_ubf_async_safe(VALUE thread, VALUE notify_fd) +{ + struct loop_ctl ctl; + + ctl.notify_fd = NUM2INT(notify_fd); + ctl.stop = 0; + + rb_nogvl(do_loop, &ctl, stop_set, &ctl, RB_NOGVL_UBF_ASYNC_SAFE); + return thread; +} + void Init_call_without_gvl(void) { rb_define_method(rb_cThread, "__runnable_sleep__", thread_runnable_sleep, 1); + rb_define_method(rb_cThread, "__ubf_async_safe__", thread_ubf_async_safe, 1); } diff --git a/ext/zlib/zlib.c b/ext/zlib/zlib.c index 1b48bb2677..f1fd2b5c8a 100644 --- a/ext/zlib/zlib.c +++ b/ext/zlib/zlib.c @@ -1011,6 +1011,7 @@ zstream_run_func(void *ptr) /* * There is no safe way to interrupt z->run->func(). + * async-signal-safe */ static void zstream_unblock_func(void *ptr) @@ -1053,8 +1054,9 @@ zstream_run(struct zstream *z, Bytef *src, long len, int flush) } loop: - err = (int)(VALUE)rb_thread_call_without_gvl(zstream_run_func, (void *)&args, - zstream_unblock_func, (void *)&args); + err = (int)(VALUE)rb_nogvl(zstream_run_func, (void *)&args, + zstream_unblock_func, (void *)&args, + RB_NOGVL_UBF_ASYNC_SAFE); if (flush != Z_FINISH && err == Z_BUF_ERROR && z->stream.avail_out > 0) { diff --git a/include/ruby/thread.h b/include/ruby/thread.h index 550f678e54..d398cc127e 100644 --- a/include/ruby/thread.h +++ b/include/ruby/thread.h @@ -21,6 +21,10 @@ extern "C" { #include "ruby/intern.h" +/* flags for rb_nogvl */ +#define RB_NOGVL_INTR_FAIL (0x1) +#define RB_NOGVL_UBF_ASYNC_SAFE (0x2) + RUBY_SYMBOL_EXPORT_BEGIN void *rb_thread_call_with_gvl(void *(*func)(void *), void *data1); @@ -30,6 +34,14 @@ void *rb_thread_call_without_gvl(void *(*func)(void *), void *data1, void *rb_thread_call_without_gvl2(void *(*func)(void *), void *data1, rb_unblock_function_t *ubf, void *data2); +/* + * XXX: unstable/unapproved - out-of-tree code should NOT not depend + * on this until it hits Ruby 2.6.1 + */ +void *rb_nogvl(void *(*func)(void *), void *data1, + rb_unblock_function_t *ubf, void *data2, + int flags); + #define RUBY_CALL_WO_GVL_FLAG_SKIP_CHECK_INTS_AFTER 0x01 #define RUBY_CALL_WO_GVL_FLAG_SKIP_CHECK_INTS_ diff --git a/test/-ext-/gvl/test_ubf_async_safe.rb b/test/-ext-/gvl/test_ubf_async_safe.rb new file mode 100644 index 0000000000..85c4a7d38e --- /dev/null +++ b/test/-ext-/gvl/test_ubf_async_safe.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true +class TestUbfAsyncSafe < Test::Unit::TestCase + def test_ubf_async_safe + skip 'need fork for single-threaded test' unless Process.respond_to?(:fork) + IO.pipe do |r, w| + pid = fork do + require '-test-/gvl/call_without_gvl' + r.close + trap(:INT) { exit!(0) } + Thread.current.__ubf_async_safe__(w.fileno) + exit!(1) + end + w.close + assert IO.select([r], nil, nil, 30), 'child did not become ready' + Process.kill(:INT, pid) + _, st = Process.waitpid2(pid) + assert_predicate st, :success?, ':INT signal triggered exit' + end + end +end diff --git a/thread.c b/thread.c index 06fd49cf2c..59935a2e1c 100644 --- a/thread.c +++ b/thread.c @@ -1421,9 +1421,10 @@ blocking_region_end(rb_thread_t *th, struct rb_blocking_region_buffer *region) } } -static void * -call_without_gvl(void *(*func)(void *), void *data1, - rb_unblock_function_t *ubf, void *data2, int fail_if_interrupted) +void * +rb_nogvl(void *(*func)(void *), void *data1, + rb_unblock_function_t *ubf, void *data2, + int flags) { void *val = 0; rb_execution_context_t *ec = GET_EC(); @@ -1436,15 +1437,22 @@ call_without_gvl(void *(*func)(void *), void *data1, data2 = th; } else if (ubf && vm_living_thread_num(th->vm) == 1) { - ubf_th = rb_thread_start_unblock_thread(); + if (RB_NOGVL_UBF_ASYNC_SAFE) { + th->vm->ubf_async_safe = 1; + } + else { + ubf_th = rb_thread_start_unblock_thread(); + } } BLOCKING_REGION(th, { val = func(data1); saved_errno = errno; - }, ubf, data2, fail_if_interrupted); + }, ubf, data2, flags & RB_NOGVL_INTR_FAIL); - if (!fail_if_interrupted) { + th->vm->ubf_async_safe = 0; + + if ((flags & RB_NOGVL_INTR_FAIL) == 0) { RUBY_VM_CHECK_INTS_BLOCKING(ec); } @@ -1546,14 +1554,14 @@ void * rb_thread_call_without_gvl2(void *(*func)(void *), void *data1, rb_unblock_function_t *ubf, void *data2) { - return call_without_gvl(func, data1, ubf, data2, TRUE); + return rb_nogvl(func, data1, ubf, data2, RB_NOGVL_INTR_FAIL); } void * rb_thread_call_without_gvl(void *(*func)(void *data), void *data1, rb_unblock_function_t *ubf, void *data2) { - return call_without_gvl(func, data1, ubf, data2, FALSE); + return rb_nogvl(func, data1, ubf, data2, 0); } VALUE diff --git a/thread_pthread.c b/thread_pthread.c index d8d3184c62..499da0b9ca 100644 --- a/thread_pthread.c +++ b/thread_pthread.c @@ -1542,6 +1542,11 @@ rb_thread_wakeup_timer_thread(int sig) if (ec) { RUBY_VM_SET_TRAP_INTERRUPT(ec); ubf_timer_arm(current); + + /* some ubfs can interrupt single-threaded process directly */ + if (vm->ubf_async_safe && mth->unblock.func) { + (mth->unblock.func)(mth->unblock.arg); + } } } } diff --git a/vm_core.h b/vm_core.h index ea59e417ef..7852ca4625 100644 --- a/vm_core.h +++ b/vm_core.h @@ -608,6 +608,9 @@ typedef struct rb_vm_struct { VALUE thgroup_default; int living_thread_num; + /* set in single-threaded processes only: */ + volatile int ubf_async_safe; + unsigned int running: 1; unsigned int thread_abort_on_exception: 1; unsigned int thread_report_on_exception: 1;