Patchwork [Ada] Race condition in Make_Independent

login
register
mail settings
Submitter Arnaud Charlet
Date Aug. 29, 2011, 2:15 p.m.
Message ID <20110829141547.GA24289@adacore.com>
Download mbox | patch
Permalink /patch/112063/
State New
Headers show

Comments

Arnaud Charlet - Aug. 29, 2011, 2:15 p.m.
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  <duff@adacore.com>

	* 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.

Patch

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.