From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x42f.google.com (mail-wr1-x42f.google.com [IPv6:2a00:1450:4864:20::42f]) by sourceware.org (Postfix) with ESMTPS id F02A0385C6D4 for ; Fri, 28 Jul 2023 07:31:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org F02A0385C6D4 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=adacore.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=adacore.com Received: by mail-wr1-x42f.google.com with SMTP id ffacd0b85a97d-317744867a6so1791166f8f.1 for ; Fri, 28 Jul 2023 00:31:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=adacore.com; s=google; t=1690529461; x=1691134261; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=vtLuYo7Psd/20RdRkcfMHsDCamKcOkdtQ1bSpg/ZxP0=; b=Fju186/aU+Xt0EGPywLRZy6FO8C+q322kWxkbjPo/uG+5oBUAAfZT1cgPie5G+q69N Ae+zfppllOKFYmYDViNFpAVCdJferpg47kl7VeVFfFk5qqtz9mwgnZnCWQmydGpfWkw4 zNcsd56lLBgUz2XsJZhyPJINp0tDftIhPH0f1jQx+EjuukrDkVA8xu1mJBJjTRk6xslk aVPt60ZVfdWdl6ZlO9JNPQ5etSX09guDmkfjRRnVRN3q5ENq5kxjVcu71BX/Th88Rkod /oM5JaZZfZDl1AsJAwsEgyk7bK62w8ZlOP92dRkzdMLlnpzFQwEzQ3SxbCMZMP9G4ais Sz9g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690529461; x=1691134261; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=vtLuYo7Psd/20RdRkcfMHsDCamKcOkdtQ1bSpg/ZxP0=; b=aawjG8+TCxi6i49ViBYoRWMSFp2l1uFZtdKHn1SdnqDT3EBDJvquILyqwKRbgXPlgB SjxbSg9bkSuctCwXYGL12TqXRaIm9NeqmONJSnehBkg9noBKgh2gAiFPzVTjqEcJe7m1 UfCR7iI4IZPx2anbaGaPrpWvFtGzgn6RFXETcbV5JQnkd2iHpoCdGG9pxF9Cn535dF7H Ttr11lhXuPxbJagTMPXN0j2u4AaD5hLs2CYt96G2Kuf9TY8g/lC6JP7ashr7HhT2H5hW uB7PHTPfWdY+wuqPDXryE4A/xNY59OggtSSnbeq0CIFseXxHz9vYzPl78BjoewGezdtw RtUA== X-Gm-Message-State: ABy/qLZtQT7fLoPbHpoZhIV5Y8yAwv79eKKB9B0DrkvtAB5RuXeoi4UV yXhJI+bG4ANcjLeOs9yOFXBR3e5M+eKseTBu5I9/hA== X-Google-Smtp-Source: APBJJlE3vs6Tt2lQyMcbkgxWqoB1pqB4+4ptYNAot0yHyJvpD59JzmXwPoO78xgESHjAoZW0fs0kiw== X-Received: by 2002:a5d:6289:0:b0:317:4755:9e3f with SMTP id k9-20020a5d6289000000b0031747559e3fmr894348wru.62.1690529460874; Fri, 28 Jul 2023 00:31:00 -0700 (PDT) Received: from localhost.localdomain ([2001:861:3382:1a90:792d:7d2b:d7e:14a3]) by smtp.gmail.com with ESMTPSA id e36-20020a5d5964000000b002fb60c7995esm4096152wri.8.2023.07.28.00.31.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 28 Jul 2023 00:31:00 -0700 (PDT) From: =?UTF-8?q?Marc=20Poulhi=C3=A8s?= To: gcc-patches@gcc.gnu.org Cc: Ronan Desplanques Subject: [COMMITTED] ada: Fix race condition in protected entry call Date: Fri, 28 Jul 2023 09:30:58 +0200 Message-Id: <20230728073058.1852740-1-poulhies@adacore.com> X-Mailer: git-send-email 2.40.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-13.7 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: From: Ronan Desplanques This patch only affects the single-entry implementation of protected objects. Before this patch, there was a race condition where a task that called an entry could put itself to sleep right after another task had executed the entry as a proxy and signalled the not-yet-waiting first task, which caused the first task to enter a deadlock. Note that this race condition has been identified and fixed before for the implementations of the run-time that live under hie/. This patch reworks the locking sequence so that it is closer to the one that's used in the multiple-entry implementation of protected objects. The code for the multiple-entry implementation is spread across multiple subprograms. To draw a parallel with the section this patch modifies, one can read the following subprograms: - System.Tasking.Protected_Objects.Operations.Protected_Entry_Call - System.Tasking.Entry_Calls.Wait_For_Completion - System.Tasking.Entry_Calls.Check_Pending_Actions_For_Entry_Call This patch also adds a comment that explicitly states the locking constraint that must hold in the affected section. gcc/ada/ * libgnarl/s-tposen.adb: Fix race condition. Add comment to justify the locking timing. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/libgnarl/s-tposen.adb | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/gcc/ada/libgnarl/s-tposen.adb b/gcc/ada/libgnarl/s-tposen.adb index 9dff6619295..a7447b9e2af 100644 --- a/gcc/ada/libgnarl/s-tposen.adb +++ b/gcc/ada/libgnarl/s-tposen.adb @@ -345,11 +345,17 @@ package body System.Tasking.Protected_Objects.Single_Entry is pragma Assert (Entry_Call.State /= Cancelled); + -- Note that we need to acquire Self_Id's lock before checking the value + -- of Entry_Call.State, even though the latter is specified as atomic + -- with a pragma. If we didn't, another task could execute the entry on + -- our behalf right between the check of Entry_Call.State and the call + -- to Wait_For_Completion, and that would cause a deadlock. + + STPO.Write_Lock (Self_Id); if Entry_Call.State /= Done then - STPO.Write_Lock (Self_Id); Wait_For_Completion (Entry_Call'Access); - STPO.Unlock (Self_Id); end if; + STPO.Unlock (Self_Id); Check_Exception (Self_Id, Entry_Call'Access); end Protected_Single_Entry_Call; -- 2.40.0