[PATCH] aio: fix newp->running data race

[Samuel Thibault <samuel dot thibault at ens-lyon dot org>]

Hello,

As reported by helgrind:

==9363== Possible data race during write of size 4 at 0x6B45D18 by thread #5
==9363== Locks held: 2, at addresses 0x557A1E0 0x6A149E0
==9363==    at 0x5375E0D: __aio_enqueue_request (in /lib/x86_64-linux-gnu/librt-2.21.so)
==9363==    by 0x53764FD: aio_write (in /lib/x86_64-linux-gnu/librt-2.21.so)

which is the write in

  if (result == 0)
    newp->running = running;

==9363== This conflicts with a previous read of size 4 by thread #6
==9363== Locks held: none
==9363==    at 0x53758FE: handle_fildes_io (in /lib/x86_64-linux-gnu/librt-2.21.so)

which is the read in 

	  /* Hopefully this request is marked as running.  */
	  assert (runp->running == allocated);

So what is happening is that __aio_enqueue_request writes newp->running
after having possibly started a thread worker to handle the request,
thus in concurrence with the thread worker, which not only reads
runp->running as shown here, but also writes to it later (and does not
take __aio_requests_mutex since it's already given a request).

But actually when we do start a new thread worker, there is no need
to set newp->running, since it was already set by the "running =
newp->running = allocated;" line along the thread creation.  So to avoid
the race we can just set newp->running in the cases where we do not
start the thread.

Samuel


    	* sysdeps/pthread/aio_misc.c (__aio_enqueue_request): Do not write
    	`running` field of `newp` when a thread was started to process it,
    	since that thread will not take `__aio_requests_mutex`, and the field
    	already has the proper value actually.

diff --git a/sysdeps/pthread/aio_misc.c b/sysdeps/pthread/aio_misc.c
index f55570d..faf139d 100644
--- a/sysdeps/pthread/aio_misc.c
+++ b/sysdeps/pthread/aio_misc.c
@@ -453,7 +453,11 @@ __aio_enqueue_request (aiocb_union *aiocbp, int operation)
 		result = 0;
 	    }
 	}
+      else
+	newp->running = running;
     }
+  else
+    newp->running = running;
 
   /* Enqueue the request in the run queue if it is not yet running.  */
   if (running == yes && result == 0)
@@ -466,9 +470,7 @@ __aio_enqueue_request (aiocb_union *aiocbp, int operation)
 	pthread_cond_signal (&__aio_new_request_notification);
     }
 
-  if (result == 0)
-    newp->running = running;
-  else
+  if (result != 0)
     {
       /* Something went wrong.  */
       __aio_free_request (newp);