From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id D1EA33858D3C for ; Wed, 9 Feb 2022 15:13:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D1EA33858D3C Received: from mail-oo1-f69.google.com (mail-oo1-f69.google.com [209.85.161.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-388-n4HuI6NYPtC6UPV7BFscDQ-1; Wed, 09 Feb 2022 10:13:39 -0500 X-MC-Unique: n4HuI6NYPtC6UPV7BFscDQ-1 Received: by mail-oo1-f69.google.com with SMTP id r12-20020a4aea8c000000b002fd5bc5d365so1650746ooh.18 for ; Wed, 09 Feb 2022 07:13:39 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Sg6fRma+r4KThH4JXVIJ9saJ9pD5nxRn6SUDEhOi5DU=; b=jsICR1pS3U5l5tUJmlfbUT2d8duVeJ6sGqGoRo43Lg2uPL7M1CQCY+WYh+nJSt5YBv fAIFIsPwrMfSGIrE0GQhK32BB9RcyAkJ4onkqHanK2FxtHrQIcYFc+RtXSSda1cD77ov nT21XVZCXGihGrRNv6avAmQxdEgsPmn7VFWwiHIQO9CCqxlDhA+iS8KA1EzY32OgyX59 yPdq3qgUQE7ixkvZiLwdZ/weKqae038QAj3zAF18Og3UrROrjZHGjOY0spJiwzfGQJpy mJyPujBrMSbLYDJxG0XTGbNmDths6iiVhi1G6SwawNU9nfLv6yPDvbXo8NYpZiKfeY+j bMwQ== X-Gm-Message-State: AOAM533RO3PMhiMFmVZzWaDI8EXYc2jrM+eUBbelEsEcZ6eZOGGGPb3g zVwMYC6u5mH4htKXNhZf4Bwc0LzI0xke+9ALXqRNNGdDHWQotSIeMrTFKJbvRFAbyMkvWwcoGVE 5VQpyuJ6nGqh8I6E8hdEw7B3jPnzWWQw= X-Received: by 2002:a9d:4e03:: with SMTP id p3mr1100568otf.299.1644419618457; Wed, 09 Feb 2022 07:13:38 -0800 (PST) X-Google-Smtp-Source: ABdhPJyr5XqVl5EgJ3v1S7q8KPdSkxboEeDO9fX1ICxf3w+UhkuVCNsMoiA5iXVaLZr7BsqtRyT9e6k6tq9BSL6smcs= X-Received: by 2002:a9d:4e03:: with SMTP id p3mr1100561otf.299.1644419618274; Wed, 09 Feb 2022 07:13:38 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Thomas Rodgers Date: Wed, 9 Feb 2022 07:13:27 -0800 Message-ID: Subject: Re: [PATCH] libstdc++: Fix deadlock in atomic wait [PR104442] To: Jonathan Wakely Cc: "libstdc++" , gcc Patches X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-6.3 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, HTML_MESSAGE, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE, URI_HEX autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: libstdc++@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libstdc++ mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 09 Feb 2022 15:13:42 -0000 Excessively enthusiastic refactoring. I expect to rewrite most of this as part of the work I'm starting now for GCC13 stage1. On Wed, Feb 9, 2022 at 2:43 AM Jonathan Wakely wrote: > On Wed, 9 Feb 2022 at 00:57, Thomas Rodgers via Libstdc++ > wrote: > > > > This issue was observed as a deadloack in > > 29_atomics/atomic/wait_notify/100334.cc on vxworks. When a wait is > > "laundered" (e.g. type T* does not suffice as a waitable address for the > > platform's native waiting primitive), the address waited is that of the > > _M_ver member of __waiter_pool_base, so several threads may wait on the > > same address for unrelated atomic's. As noted in the PR, the > > implementation correctly exits the wait for the thread who's data > > changed, but not for any other threads waiting on the same address. > > > > As noted in the PR the __waiter::_M_do_wait_v member was correctly > exiting > > but the other waiters were not reloaded the value of _M_ver before > > re-entering the wait. > > > > Moving the spin call inside the loop accomplishes this, and is > > consistent with the predicate accepting version of __waiter::_M_do_wait. > > There is a change to the memory order in _S_do_spin_v which is not > described in the commit msg or the changelog. Is that unintentional? > > (Aside: why do we even have _S_do_spin_v, it's called in exactly one > place, so could just be inlined into _M_do_spin_v, couldn't it?) > >