From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-4.0 required=3.0 tests=ALL_TRUSTED,AWL,BAYES_00 shortcircuit=no autolearn=ham autolearn_force=no version=3.4.0 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 82987208DB; Mon, 28 Aug 2017 23:26:57 +0000 (UTC) Date: Mon, 28 Aug 2017 23:26:57 +0000 From: Eric Wong To: spew@80x24.org Subject: [PATCH v2?] thread_pthread.c: avoid infinite loop Message-ID: <20170828232657.GA22848@dcvr> References: <20170809232533.14932-1-e@80x24.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170809232533.14932-1-e@80x24.org> List-Id: * thread_pthread.c (struct timer_thread_pipe): make `writing' volatile (rb_thread_create_timer_thread): zero `writing' on create (rb_thread_wakeup_timer_thread): check ownership before incrementing (rb_thread_wakeup_timer_thread_low): ditto Likely overkill fix for https://bugs.ruby-lang.org/issues/13794 I don't think zero-ing `writing' is necessary, because the ownership check before incrementing should be sufficient. --- thread_pthread.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/thread_pthread.c b/thread_pthread.c index 242b48f15d..8299224ca0 100644 --- a/thread_pthread.c +++ b/thread_pthread.c @@ -1265,7 +1265,7 @@ static struct { /* volatile for signal handler use: */ volatile rb_pid_t owner_process; - rb_atomic_t writing; + volatile rb_atomic_t writing; } timer_thread_pipe = { {-1, -1}, {-1, -1}, /* low priority */ @@ -1318,17 +1318,21 @@ void rb_thread_wakeup_timer_thread(void) { /* must be safe inside sighandler, so no mutex */ - ATOMIC_INC(timer_thread_pipe.writing); - rb_thread_wakeup_timer_thread_fd(&timer_thread_pipe.normal[1]); - ATOMIC_DEC(timer_thread_pipe.writing); + if (timer_thread_pipe.owner_process == getpid()) { + ATOMIC_INC(timer_thread_pipe.writing); + rb_thread_wakeup_timer_thread_fd(&timer_thread_pipe.normal[1]); + ATOMIC_DEC(timer_thread_pipe.writing); + } } static void rb_thread_wakeup_timer_thread_low(void) { - ATOMIC_INC(timer_thread_pipe.writing); - rb_thread_wakeup_timer_thread_fd(&timer_thread_pipe.low[1]); - ATOMIC_DEC(timer_thread_pipe.writing); + if (timer_thread_pipe.owner_process == getpid()) { + ATOMIC_INC(timer_thread_pipe.writing); + rb_thread_wakeup_timer_thread_fd(&timer_thread_pipe.low[1]); + ATOMIC_DEC(timer_thread_pipe.writing); + } } /* VM-dependent API is not available for this function */ @@ -1670,7 +1674,8 @@ rb_thread_create_timer_thread(void) return; } - /* validate pipe on this process */ + /* validate pipe on this process, volatile since order matters: */ + timer_thread_pipe.writing = 0; timer_thread_pipe.owner_process = getpid(); timer_thread.created = 1; } -- EW