From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Mark H Weaver Newsgroups: gmane.lisp.guile.bugs Subject: bug#32786: (system* ...) call somehow interferes with atomic-box on armv7l Date: Fri, 05 Oct 2018 19:22:13 -0400 Message-ID: <87lg7cf07u.fsf@netris.org> References: <20180920171306.GA9185@k> <87fty2v8xo.fsf@netris.org> <20180922043938.GA4539@blacky> <87sh1vcwws.fsf@netris.org> <20181005194357.GA542@k> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Trace: blaine.gmane.org 1538781674 21268 195.159.176.226 (5 Oct 2018 23:21:14 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Fri, 5 Oct 2018 23:21:14 +0000 (UTC) User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) Cc: 32786-done@debbugs.gnu.org To: =?UTF-8?Q?=D0=9C=D0=B8=D1=85=D0=B0=D0=B8=D0=BB_?= =?UTF-8?Q?=D0=91=D0=B0=D1=85=D1=82=D0=B5=D1=80=D0=B5=D0=B2?= Original-X-From: bug-guile-bounces+guile-bugs=m.gmane.org@gnu.org Sat Oct 06 01:21:10 2018 Return-path: Envelope-to: guile-bugs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1g8ZPB-0005Lq-Vy for guile-bugs@m.gmane.org; Sat, 06 Oct 2018 01:21:10 +0200 Original-Received: from localhost ([::1]:37271 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1g8ZRD-0007nT-Al for guile-bugs@m.gmane.org; Fri, 05 Oct 2018 19:23:15 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:54062) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1g8ZR6-0007lu-MG for bug-guile@gnu.org; Fri, 05 Oct 2018 19:23:09 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1g8ZR3-0002yh-EH for bug-guile@gnu.org; Fri, 05 Oct 2018 19:23:08 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:33995) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1g8ZR0-0002tu-4b for bug-guile@gnu.org; Fri, 05 Oct 2018 19:23:03 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1g8ZQz-0005uq-Ru for bug-guile@gnu.org; Fri, 05 Oct 2018 19:23:01 -0400 Resent-From: Mark H Weaver Original-Sender: "Debbugs-submit" Resent-To: bug-guile@gnu.org Resent-Date: Fri, 05 Oct 2018 23:23:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: cc-closed 32786 X-GNU-PR-Package: guile X-GNU-PR-Keywords: Mail-Followup-To: 32786@debbugs.gnu.org, mhw@netris.org, mob@k.imm.uran.ru Original-Received: via spool by 32786-done@debbugs.gnu.org id=D32786.153878175522687 (code D ref 32786); Fri, 05 Oct 2018 23:23:01 +0000 Original-Received: (at 32786-done) by debbugs.gnu.org; 5 Oct 2018 23:22:35 +0000 Original-Received: from localhost ([127.0.0.1]:38253 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1g8ZQZ-0005tr-90 for submit@debbugs.gnu.org; Fri, 05 Oct 2018 19:22:35 -0400 Original-Received: from world.peace.net ([64.112.178.59]:38276) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1g8ZQW-0005tc-Ck for 32786-done@debbugs.gnu.org; Fri, 05 Oct 2018 19:22:34 -0400 Original-Received: from mhw by world.peace.net with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.89) (envelope-from ) id 1g8ZQP-000383-Vm; Fri, 05 Oct 2018 19:22:26 -0400 In-Reply-To: <20181005194357.GA542@k> ("=?UTF-8?Q?=D0=9C=D0=B8=D1=85=D0=B0=D0=B8=D0=BB_?= =?UTF-8?Q?=D0=91=D0=B0=D1=85=D1=82=D0=B5=D1=80=D0=B5=D0=B2?="'s message of "Sat, 6 Oct 2018 00:43:57 +0500") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.43 X-BeenThere: bug-guile@gnu.org List-Id: "Bug reports for GUILE, GNU's Ubiquitous Extension Language" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-guile-bounces+guile-bugs=m.gmane.org@gnu.org Original-Sender: "bug-guile" Xref: news.gmane.org gmane.lisp.guile.bugs:9180 Archived-At: Hi, =D0=9C=D0=B8=D1=85=D0=B0=D0=B8=D0=BB =D0=91=D0=B0=D1=85=D1=82=D0=B5=D1=80= =D0=B5=D0=B2 writes: > I've tested the patch (i've applied it to the v2.2.4 source code). The > test sample works now as expected with both test =C2=ABpayloads=C2=BB: (s= leep > N-seconds) and (system* "any" "command" "here") calls. Thanks for letting us know the results of your testing, this is very helpful. I just pushed a similar fix (same code, improved comments) to the stable-2.2 branch, commit 2d09e0513fc11e2305077ba3653f6e4c2f266ddb, which will be in the upcoming guile-2.2.5 release. Thanks, Mark > > Thanks for fixing it. > > - MB. Respectfully > > On Thu, Sep 27, 2018 at 01:49:23AM -0400, Mark H Weaver wrote: >> Hi, >>=20 >> Thanks for the additional details. I was able to reproduce the bug, and >> I believe I now see the problem. >>=20 >> 'atomic-box-compare-and-swap!' is implemented using >> 'atomic_compare_exchange_weak' (if available), but neglects to handle >> the case where 'atomic_compare_exchange_weak' may spuriously fail. In >> that case, the box is left unchanged, although the observed value was >> equal to the expected value. >>=20 >> What's happening here is that the 'atomic-box-compare-and-swap!' in >> 'sleep-loop' fails spuriously, leaving the box in state #:accepted >> although it returns #:accepted as its result. When the main loop >> discovers this, it changes the state to #:need-to-sleep, although the >> thread has already ended. >>=20 >> To confirm this hypothesis, I added a print statement to the main loop >> showing the state of the box that it observed during the protocol >> exchange, and indeed it sees #:accepted the first time it checks, and >> #:need-to-sleep in all later iterations. >>=20 >> I've attached a proposed patch that I hope will fix this problem. If >> you'd be willing to test it, I'd be grateful, but otherwise I'll try to >> test it in the next week or so. My access to armv7l boxes is somewhat >> limited. >>=20 >> Thanks for this report. >>=20 >> Mark >>=20 >>=20 > >> From 958d37686a9ac65f48cb2ca32a341cf182c24b5a Mon Sep 17 00:00:00 2001 >> From: Mark H Weaver >> Date: Thu, 27 Sep 2018 01:00:11 -0400 >> Subject: [PATCH] UNTESTED: Fix 'atomic-box-compare-and-swap!'. >>=20 >> Fixes . >>=20 >> 'scm_atomic_compare_and_swap_scm' uses 'atomic_compare_exchange_weak' if >> it's available on the host platform. 'atomic_compare_exchange_weak' may >> fail spuriously, leaving the atomic object unchanged even when it >> contained the expected value. 'atomic-box-compare-and-swap!' uses >> 'scm_atomic_compare_and_swap_scm' without checking its return value, and >> therefore may return the expected value while leaving the box unchanged. >> This contradicts the documentation, which explicitly states that "you >> can know if the swap worked by checking if the return value is 'eq?' to >> EXPECTED.". >>=20 >> * libguile/atomic.c (scm_atomic_box_compare_and_swap_x): If >> 'scm_atomic_compare_and_swap_scm' returns false and the observed value >> is equal to 'expected', then try again. >> * libguile/vm-engine.c (atomic-box-compare-and-swap!): Ditto. >> --- >> libguile/atomic.c | 13 +++++++++---- >> libguile/vm-engine.c | 13 ++++++++----- >> 2 files changed, 17 insertions(+), 9 deletions(-) >>=20 >> diff --git a/libguile/atomic.c b/libguile/atomic.c >> index 950874030..504643912 100644 >> --- a/libguile/atomic.c >> +++ b/libguile/atomic.c >> @@ -1,4 +1,4 @@ >> -/* Copyright (C) 2016 Free Software Foundation, Inc. >> +/* Copyright (C) 2016, 2018 Free Software Foundation, Inc. >> * >> * This library is free software; you can redistribute it and/or >> * modify it under the terms of the GNU Lesser General Public License >> @@ -95,10 +95,15 @@ SCM_DEFINE (scm_atomic_box_compare_and_swap_x, >> "if the return value is @code{eq?} to @var{expected}.") >> #define FUNC_NAME s_scm_atomic_box_compare_and_swap_x >> { >> + SCM result =3D expected; >> + >> SCM_VALIDATE_ATOMIC_BOX (1, box); >> - scm_atomic_compare_and_swap_scm (scm_atomic_box_loc (box), >> - &expected, desired); >> - return expected; >> + while (!scm_atomic_compare_and_swap_scm (scm_atomic_box_loc (box), >> + &result, desired) >> + && scm_is_eq (result, expected)) >> + { >> + } >> + return result; >> } >> #undef FUNC_NAME >>=20=20 >> diff --git a/libguile/vm-engine.c b/libguile/vm-engine.c >> index 19ff3e498..9650e33eb 100644 >> --- a/libguile/vm-engine.c >> +++ b/libguile/vm-engine.c >> @@ -3868,16 +3868,19 @@ VM_NAME (scm_i_thread *thread, struct scm_vm *vp, >> { >> scm_t_uint16 dst, box; >> scm_t_uint32 expected, desired; >> - SCM scm_box, scm_expected; >> + SCM scm_box, scm_expected, scm_result; >> UNPACK_12_12 (op, dst, box); >> UNPACK_24 (ip[1], expected); >> UNPACK_24 (ip[2], desired); >> scm_box =3D SP_REF (box); >> VM_VALIDATE_ATOMIC_BOX (scm_box, "atomic-box-compare-and-swap!"); >> - scm_expected =3D SP_REF (expected); >> - scm_atomic_compare_and_swap_scm (scm_atomic_box_loc (scm_box), >> - &scm_expected, SP_REF (desired)); >> - SP_SET (dst, scm_expected); >> + scm_result =3D scm_expected =3D SP_REF (expected); >> + while (!scm_atomic_compare_and_swap_scm (scm_atomic_box_loc (scm_= box), >> + &scm_result, SP_REF (des= ired)) >> + && scm_is_eq (scm_result, scm_expected)) >> + { >> + } >> + SP_SET (dst, scm_result); >> NEXT (3); >> } >>=20=20 >> --=20 >> 2.19.0 >>=20