This is the mail archive of the
mailing list for the glibc project.
Re: [PATCH] aio: fix newp->running data race
- From: Samuel Thibault <samuel dot thibault at ens-lyon dot org>
- To: Torvald Riegel <triegel at redhat dot com>
- Cc: GNU C Library <libc-alpha at sourceware dot org>
- Date: Thu, 3 Aug 2017 18:42:17 +0200
- Subject: Re: [PATCH] aio: fix newp->running data race
- Authentication-results: sourceware.org; auth=none
- References: <20160504140217.GZ2861@var.bordeaux.inria.fr> <20160708142810.GF13644@var.bordeaux.inria.fr> <email@example.com> <20160817124929.GW5802@var>
Samuel Thibault, on mer. 17 août 2016 14:49:29 +0200, wrote:
> Torvald Riegel, on Fri 08 Jul 2016 18:05:15 +0200, wrote:
> > On Fri, 2016-07-08 at 16:28 +0200, Samuel Thibault wrote:
> > > Ping?
> > I've just looked quickly at the patch. Fixes for concurrency issues
> > should be accompanied with detailed comments, so that other developers
> > can easily grasp how synchronization is supposed to work.
> Ok. How about this?
* 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.
* sysdeps/pthread/aio_misc.h: Add a notes about concurrency of
diff --git a/sysdeps/pthread/aio_misc.c b/sysdeps/pthread/aio_misc.c
index f55570d..00b2feb 100644
@@ -435,7 +435,9 @@ __aio_enqueue_request (aiocb_union *aiocbp, int operation)
if (result == 0)
/* We managed to enqueue the request. All errors which can
happen now can be recognized by calls to `aio_return' and
- `aio_error'. */
+ `aio_error'. From here, newp must not be modified any more
+ since a helper thread could be already working on it.
@@ -453,7 +455,11 @@ __aio_enqueue_request (aiocb_union *aiocbp, int operation)
result = 0;
+ newp->running = running;
+ newp->running = running;
/* Enqueue the request in the run queue if it is not yet running. */
if (running == yes && result == 0)
@@ -466,9 +472,7 @@ __aio_enqueue_request (aiocb_union *aiocbp, int operation)
- if (result == 0)
- newp->running = running;
+ if (result != 0)
/* Something went wrong. */
diff --git a/sysdeps/pthread/aio_misc.h b/sysdeps/pthread/aio_misc.h
index e042998..66ef3b1 100644
@@ -72,6 +72,18 @@ enum
+/* CONCURRENCY NOTES:
+ __aio_requests_mutex only protects the request list. The requests
+ themselves are not protected, since the producer is alone producing it
+ before pushing it to the list, and the consumer is alone working on it after
+ popping from the list.
+ Notably, when __aio_create_helper_thread is used, it directly gives the
+ request to the thread, and thus no lock is taken. Consequently, the
+ producer must not access the request after calling
+ __aio_create_helper_thread since the consumer will immediately work on it
+ without taking __aio_requests_mutex. */
/* Used to queue requests.. */