* [Ada] Fix obscure race condition in term alts
@ 2011-08-04 10:01 Arnaud Charlet
0 siblings, 0 replies; only message in thread
From: Arnaud Charlet @ 2011-08-04 10:01 UTC (permalink / raw)
To: gcc-patches; +Cc: Bob Duff
[-- Attachment #1: Type: text/plain, Size: 742 bytes --]
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 <duff@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.
[-- Attachment #2: difs --]
[-- Type: text/plain, Size: 4048 bytes --]
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
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2011-08-04 10:01 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-04 10:01 [Ada] Fix obscure race condition in term alts Arnaud Charlet
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).