From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-x332.google.com (mail-wm1-x332.google.com [IPv6:2a00:1450:4864:20::332]) by sourceware.org (Postfix) with ESMTPS id 17F763839C5A for ; Tue, 20 Apr 2021 23:40:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 17F763839C5A Received: by mail-wm1-x332.google.com with SMTP id p10-20020a1c544a0000b02901387e17700fso238491wmi.2 for ; Tue, 20 Apr 2021 16:40:31 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=zVjXRMr7bDyw8CcNwY/YrBgglgnWX62+m1IIpX9vs6s=; b=TGmJVZkoTTao5z+x6qBnAWFhfBRrhT7yUQR09Ct240t+TtK1/CWVAjo8pevI3xi15O eSUlL/DseST1JJLJBuxlS9ncWC9bd9aZIt5SPzeMA0H8oRtdYlN/xCux0Pbdl6WTPmxK SKH4H8yPyGogCghdTt7SOta0pbQvKFMdTT3hCWujjGGpSG0l2bKyw08EUL/KJwMNqlz+ pgjABav1WhawHmEUzBdTW4JuOt31HMJLB2mU3kpZcp5C/E/LXJGg5+72TfT4l/Mcxti5 cVHQ/pYqK45GVt7CLvQ41fPNdMH1xEfG1b8efGteLg9qKmRrHID9w//06nKKu7sBzTWB 3Puw== X-Gm-Message-State: AOAM530qzjYTFF4MNxOCh6F35cMFo7R1hUQnCcfAIUOcotwm6lmh7Q96 Ks2kqOAoOASZnbjLLZr41IPiX7k6v+HHHi+iHh3P5g== X-Google-Smtp-Source: ABdhPJxb9IvoHf/iVzUskMv40+90lMuzeRFL3/8OYQf29pumyED6INcpyO6YvRJqVxq/2JfWk3w0gbe+HT/2/ePcYP8= X-Received: by 2002:a7b:c153:: with SMTP id z19mr6897501wmi.5.1618962029915; Tue, 20 Apr 2021 16:40:29 -0700 (PDT) MIME-Version: 1.0 References: <20210417172258.2788076-1-vitalybuka@google.com> In-Reply-To: From: Paul Pluzhnikov Date: Tue, 20 Apr 2021 16:40:03 -0700 Message-ID: Subject: Re: [PATCH] stdlib: Fix data race in __run_exit_handlers To: Vitaly Buka Cc: GLIBC Devel Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-18.4 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, ENV_AND_HDR_SPF_MATCH, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, USER_IN_DEF_DKIM_WL, USER_IN_DEF_SPF_WL autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 20 Apr 2021 23:40:41 -0000 On Tue, Apr 20, 2021 at 3:51 PM Vitaly Buka wrote: >> > +static void *cb_arg = NULL; >> > +static void >> > +add_handlers (void) >> > +{ >> > + int n = 10; >> > + for (int i = 0; i < n; ++i) >> > + __cxa_atexit (&atexit_cb, ++cb_arg, 0); >> >> add_handlers() is called from many threads. This code appears to race on cb_arg. > > > We don't have a data race as add_handlers is called from a single background thread. > Previous patches had more threads but they didn't help to reproduce the issues. Thanks. I think cb_arg can be moved into add_handlers() and doesn't have to be static anymore (if it were a local, there wouldn't be a question of a race in the first place). I've looked at the code and the first data race description at the start of this thread. I agree that this is the right fix for it. I have not yet understood the second interaction (between __run_exit_handlers and __new_exitfn), but I am not sure I really need to: the patch seems correct. One other change I would make is to move the unlock before PTR_DEMANGLE (since it doesn't use any of the data guarded by this lock). -- Paul Pluzhnikov