This patch is fixes an obscure race condition in the implementation of terminate alternatives. No small test case is available.
Tested on x86_64-pc-linux-gnu, committed on trunk 2011-08-04 Bob Duff <d...@adacore.com> * s-tasren.adb (Task_Do_Or_Queue): Previous code was reading Acceptor.Terminate_Alternative without locking Acceptor, which causes a race condition whose symptom is to fail to lock Parent. That, in turn, causes Parent.Awake_Count to be accessed without locking Parent, which causes another race condition whose symptom is that Parent.Awake_Count can be off by 1 (either too high or too low). The solution is to lock Parent unconditionally, and then lock Acceptor, before reading Acceptor.Terminate_Alternative.
Index: s-tasren.adb =================================================================== --- s-tasren.adb (revision 177274) +++ s-tasren.adb (working copy) @@ -6,7 +6,7 @@ -- -- -- B o d y -- -- -- --- 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- -- @@ -1077,7 +1077,6 @@ Old_State : constant Entry_Call_State := Entry_Call.State; Acceptor : constant Task_Id := Entry_Call.Called_Task; Parent : constant Task_Id := Acceptor.Common.Parent; - Parent_Locked : Boolean := False; Null_Body : Boolean; begin @@ -1105,25 +1104,24 @@ -- record for another call. -- We rely on the Caller's lock for call State mod's. - -- We can't lock Acceptor.Parent while holding Acceptor, - -- so lock it in advance if we expect to need to lock it. + -- If Acceptor.Terminate_Alternative is True, we need to lock Parent and + -- Acceptor, in that order; otherwise, we only need a lock on + -- Acceptor. However, we can't check Acceptor.Terminate_Alternative + -- until Acceptor is locked. Therefore, we need to lock both. Attempts + -- to avoid locking Parent tend to result in race conditions. It would + -- work to unlock Parent immediately upon finding + -- Acceptor.Terminate_Alternative to be False, but that violates the + -- rule of properly nested locking (see System.Tasking). - if Acceptor.Terminate_Alternative then - STPO.Write_Lock (Parent); - Parent_Locked := True; - end if; - + STPO.Write_Lock (Parent); STPO.Write_Lock (Acceptor); -- If the acceptor is not callable, abort the call and return False if not Acceptor.Callable then STPO.Unlock (Acceptor); + STPO.Unlock (Parent); - if Parent_Locked then - STPO.Unlock (Parent); - end if; - pragma Assert (Entry_Call.State < Done); -- In case we are not the caller, set up the caller @@ -1186,11 +1184,8 @@ STPO.Wakeup (Acceptor, Acceptor_Sleep); STPO.Unlock (Acceptor); + STPO.Unlock (Parent); - if Parent_Locked then - STPO.Unlock (Parent); - end if; - STPO.Write_Lock (Entry_Call.Self); Initialization.Wakeup_Entry_Caller (Self_ID, Entry_Call, Done); @@ -1207,10 +1202,7 @@ end if; STPO.Unlock (Acceptor); - - if Parent_Locked then - STPO.Unlock (Parent); - end if; + STPO.Unlock (Parent); end if; return True; @@ -1236,11 +1228,8 @@ and then Entry_Call.Cancellation_Attempted) then STPO.Unlock (Acceptor); + STPO.Unlock (Parent); - if Parent_Locked then - STPO.Unlock (Parent); - end if; - STPO.Write_Lock (Entry_Call.Self); pragma Assert (Entry_Call.State >= Was_Abortable); @@ -1261,11 +1250,8 @@ New_State (Entry_Call.With_Abort, Entry_Call.State); STPO.Unlock (Acceptor); + STPO.Unlock (Parent); - if Parent_Locked then - STPO.Unlock (Parent); - end if; - if Old_State /= Entry_Call.State and then Entry_Call.State = Now_Abortable and then Entry_Call.Mode /= Simple_Call