From: Amos Jeffries <squid3@treenet.co.nz>
Date: Thu, 29 Nov 2012 11:28:05 +0000 (-0700)
Subject: Bug 3189: AIO thread race on pipe() initialization
X-Git-Tag: SQUID_3_1_22~4
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=87feb56f75c9d059c3b03aab68c446920aa73f1a;p=thirdparty%2Fsquid.git

Bug 3189: AIO thread race on pipe() initialization
---

diff --git a/src/CommIO.h b/src/CommIO.h
index 4577ab1eea..33801f160b 100644
--- a/src/CommIO.h
+++ b/src/CommIO.h
@@ -9,25 +9,25 @@ class CommIO
 public:
     static inline void NotifyIOCompleted();
     static void ResetNotifications();
-    static void Initialise();
+    static void Initialize();
     static void NotifyIOClose();
 
 private:
     static void NULLFDHandler(int, void *);
     static void FlushPipe();
-    static bool Initialised;
+    static bool Initialized;
     static bool DoneSignalled;
     static int DoneFD;
     static int DoneReadFD;
 };
 
-
-/* Inline code. TODO: make structued approach to inlining */
+/* Inline code. TODO: make structured approach to inlining */
 void
 CommIO::NotifyIOCompleted()
 {
-    if (!Initialised)
-        Initialise();
+    if (!Initialized) {
+        fatalf("Disk Threads I/O pipes not initialized before first use.");
+    }
 
     if (!DoneSignalled) {
         DoneSignalled = true;
diff --git a/src/DiskIO/DiskThreads/aiops.cc b/src/DiskIO/DiskThreads/aiops.cc
index 25ecbb349e..e63746134c 100644
--- a/src/DiskIO/DiskThreads/aiops.cc
+++ b/src/DiskIO/DiskThreads/aiops.cc
@@ -309,6 +309,10 @@ squidaio_init(void)
 
     done_queue.blocked = 0;
 
+    // Initialize the thread I/O pipes before creating any threads
+    // see bug 3189 comment 5 about race conditions.
+    CommIO::Initialize();
+
     /* Create threads and get them to sit in their wait loop */
     squidaio_thread_pool = memPoolCreate("aio_thread", sizeof(squidaio_thread_t));
 
diff --git a/src/comm.cc b/src/comm.cc
index aff238b425..97cb50010e 100644
--- a/src/comm.cc
+++ b/src/comm.cc
@@ -2362,19 +2362,23 @@ comm_accept_try(int fd, void *)
     fdc_table[fd].acceptNext();
 }
 
-void CommIO::Initialise()
+void
+CommIO::Initialize()
 {
+    if (CommIO::Initialized)
+        return;
+
     /* Initialize done pipe signal */
     int DonePipe[2];
     if (pipe(DonePipe)) {}
     DoneFD = DonePipe[1];
     DoneReadFD = DonePipe[0];
-    fd_open(DoneReadFD, FD_PIPE, "async-io completetion event: main");
-    fd_open(DoneFD, FD_PIPE, "async-io completetion event: threads");
+    fd_open(DoneReadFD, FD_PIPE, "async-io completion event: main");
+    fd_open(DoneFD, FD_PIPE, "async-io completion event: threads");
     commSetNonBlocking(DoneReadFD);
     commSetNonBlocking(DoneFD);
     commSetSelect(DoneReadFD, COMM_SELECT_READ, NULLFDHandler, NULL, 0);
-    Initialised = true;
+    Initialized = true;
 }
 
 void CommIO::NotifyIOClose()
@@ -2385,10 +2389,10 @@ void CommIO::NotifyIOClose()
     close(DoneReadFD);
     fd_close(DoneFD);
     fd_close(DoneReadFD);
-    Initialised = false;
+    Initialized = false;
 }
 
-bool CommIO::Initialised = false;
+bool CommIO::Initialized = false;
 bool CommIO::DoneSignalled = false;
 int CommIO::DoneFD = -1;
 int CommIO::DoneReadFD = -1;
diff --git a/src/tests/stub_CommIO.cc b/src/tests/stub_CommIO.cc
index dfb38ec954..6112519519 100644
--- a/src/tests/stub_CommIO.cc
+++ b/src/tests/stub_CommIO.cc
@@ -1,7 +1,7 @@
 #include "squid.h"
 #include "CommIO.h"
 
-bool CommIO::Initialised = false;
+bool CommIO::Initialized = false;
 bool CommIO::DoneSignalled = false;
 int CommIO::DoneFD = -1;
 int CommIO::DoneReadFD = -1;
@@ -13,7 +13,7 @@ CommIO::ResetNotifications()
 }
 
 void
-CommIO::Initialise()
+CommIO::Initialize()
 {
     fatal("Not Implemented");
 }