From patchwork Mon Aug 29 14:15:47 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnaud Charlet X-Patchwork-Id: 112063 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id EDC44B6F90 for ; Tue, 30 Aug 2011 00:16:10 +1000 (EST) Received: (qmail 8564 invoked by alias); 29 Aug 2011 14:16:07 -0000 Received: (qmail 8552 invoked by uid 22791); 29 Aug 2011 14:16:04 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 29 Aug 2011 14:15:48 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 367262BAF18; Mon, 29 Aug 2011 10:15:47 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id vidYpZlmXpUI; Mon, 29 Aug 2011 10:15:47 -0400 (EDT) Received: from kwai.gnat.com (kwai.gnat.com [205.232.38.4]) by rock.gnat.com (Postfix) with ESMTP id 1BBD42BAF0A; Mon, 29 Aug 2011 10:15:47 -0400 (EDT) Received: by kwai.gnat.com (Postfix, from userid 4192) id 19AC192A55; Mon, 29 Aug 2011 10:15:47 -0400 (EDT) Date: Mon, 29 Aug 2011 10:15:47 -0400 From: Arnaud Charlet To: gcc-patches@gcc.gnu.org Cc: Bob Duff Subject: [Ada] Race condition in Make_Independent Message-ID: <20110829141547.GA24289@adacore.com> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.20 (2009-06-14) Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Make_Independent decrements Wait_Count, which caused a race condition -- if the timing is just wrong, it causes Make_Passive to find Wait_Count = 0, which eventually causes the program to hang. This patch prevents hanging by decrementing Wait_Count only if nonzero. No test is available, because it's large, and only fails intermittently. Tested on x86_64-pc-linux-gnu, committed on trunk 2011-08-29 Bob Duff * s-tasuti.adb (Make_Passive): Work around race condition in Make_Independent, which can cause Wait_Count to be zero. So instead of asserting that Wait_Count > 0, and then decrementing it, decrement it only if Wait_Count > 0. * s-taskin.ads (Wait_Count, Alive_Count, Awake_Count): All of these should be nonnegative, so declare them Natural instead of Integer. Index: s-tasuti.adb =================================================================== --- s-tasuti.adb (revision 178155) +++ s-tasuti.adb (working copy) @@ -6,7 +6,7 @@ -- -- -- B o d y -- -- -- --- Copyright (C) 1992-2009, Free Software Foundation, Inc. -- +-- Copyright (C) 1992-2011, Free Software Foundation, Inc. -- -- -- -- GNARL is free software; you can redistribute it and/or modify it under -- -- terms of the GNU General Public License as published by the Free Soft- -- @@ -504,12 +504,14 @@ (Debug.Trace (Self_ID, "Make_Passive: Phase 1, parent waiting", 'M')); - -- If parent is in Master_Completion_Sleep, it - -- cannot be on a terminate alternative, hence - -- it cannot have Awake_Count of zero. + -- If parent is in Master_Completion_Sleep, it cannot be on a + -- terminate alternative, hence it cannot have Wait_Count of + -- zero. ???Except that the race condition in Make_Independent can + -- cause Wait_Count to be zero, so we need to check for that. - pragma Assert (P.Common.Wait_Count > 0); - P.Common.Wait_Count := P.Common.Wait_Count - 1; + if P.Common.Wait_Count > 0 then + P.Common.Wait_Count := P.Common.Wait_Count - 1; + end if; if P.Common.Wait_Count = 0 then Wakeup (P, Master_Completion_Sleep); Index: s-taskin.ads =================================================================== --- s-taskin.ads (revision 178155) +++ s-taskin.ads (working copy) @@ -6,7 +6,7 @@ -- -- -- S p e c -- -- -- --- Copyright (C) 1992-2010, Free Software Foundation, Inc. -- +-- Copyright (C) 1992-2011, Free Software Foundation, Inc. -- -- -- -- GNARL is free software; you can redistribute it and/or modify it under -- -- terms of the GNU General Public License as published by the Free Soft- -- @@ -566,7 +566,7 @@ -- Protection: Set by Activator before Self is activated, and only read -- and modified by Self after that. - Wait_Count : Integer; + Wait_Count : Natural; -- This count is used by a task that is waiting for other tasks. At all -- other times, the value should be zero. It is used differently in -- several different states. Since a task cannot be in more than one of @@ -942,13 +942,13 @@ -- not write this field until the master is complete, the -- synchronization should be adequate to prevent races. - Alive_Count : Integer := 0; + Alive_Count : Natural := 0; -- Number of tasks directly dependent on this task (including itself) -- that are still "alive", i.e. not terminated. -- -- Protection: Self.L - Awake_Count : Integer := 0; + Awake_Count : Natural := 0; -- Number of tasks directly dependent on this task (including itself) -- still "awake", i.e., are not terminated and not waiting on a -- terminate alternative.