From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Fri, 21 Feb 2014 15:45:01 +0000 (-0700)
Subject: Fixed stalled concurrent rock store reads by insuring their ID uniqueness.
X-Git-Tag: SQUID_3_5_0_1~352
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=28bd45ba856888db6a49c56f596aaf1af8da880c;p=thirdparty%2Fsquid.git

Fixed stalled concurrent rock store reads by insuring their ID uniqueness.

Added a check to prevent similar bugs from occurring in the future.
---

diff --git a/src/DiskIO/IpcIo/IpcIoFile.cc b/src/DiskIO/IpcIo/IpcIoFile.cc
index 7307727d85..d60d73f908 100644
--- a/src/DiskIO/IpcIo/IpcIoFile.cc
+++ b/src/DiskIO/IpcIo/IpcIoFile.cc
@@ -307,9 +307,11 @@ IpcIoFile::ioInProgress() const
 
 /// track a new pending request
 void
-IpcIoFile::trackPendingRequest(IpcIoPendingRequest *const pending)
+IpcIoFile::trackPendingRequest(const unsigned int id, IpcIoPendingRequest *const pending)
 {
-    newerRequests->insert(std::make_pair(lastRequestId, pending));
+    const std::pair<RequestMap::iterator,bool> result =
+        newerRequests->insert(std::make_pair(id, pending));
+    Must(result.second); // failures means that id was not unique
     if (!timeoutCheckScheduled)
         scheduleTimeoutCheck();
 }
@@ -319,6 +321,7 @@ void
 IpcIoFile::push(IpcIoPendingRequest *const pending)
 {
     // prevent queue overflows: check for responses to earlier requests
+    // warning: this call may result in indirect push() recursion
     HandleResponses("before push");
 
     debugs(47, 7, HERE);
@@ -328,6 +331,8 @@ IpcIoFile::push(IpcIoPendingRequest *const pending)
 
     IpcIoMsg ipcIo;
     try {
+        if (++lastRequestId == 0) // don't use zero value as requestId
+            ++lastRequestId;
         ipcIo.requestId = lastRequestId;
         ipcIo.start = current_time;
         if (pending->readRequest) {
@@ -351,7 +356,7 @@ IpcIoFile::push(IpcIoPendingRequest *const pending)
 
         if (queue->push(diskId, ipcIo))
             Notify(diskId); // must notify disker
-        trackPendingRequest(pending);
+        trackPendingRequest(ipcIo.requestId, pending);
     } catch (const Queue::Full &) {
         debugs(47, DBG_IMPORTANT, "Worker I/O push queue overflow: " <<
                SipcIo(KidIdentifier, ipcIo, diskId)); // TODO: report queue len
@@ -609,9 +614,6 @@ IpcIoMsg::IpcIoMsg():
 IpcIoPendingRequest::IpcIoPendingRequest(const IpcIoFile::Pointer &aFile):
         file(aFile), readRequest(NULL), writeRequest(NULL)
 {
-    Must(file != NULL);
-    if (++file->lastRequestId == 0) // don't use zero value as requestId
-        ++file->lastRequestId;
 }
 
 void
diff --git a/src/DiskIO/IpcIo/IpcIoFile.h b/src/DiskIO/IpcIo/IpcIoFile.h
index ed84869c2d..fcf91e5b20 100644
--- a/src/DiskIO/IpcIo/IpcIoFile.h
+++ b/src/DiskIO/IpcIo/IpcIoFile.h
@@ -85,7 +85,7 @@ protected:
     bool canWait() const;
 
 private:
-    void trackPendingRequest(IpcIoPendingRequest *const pending);
+    void trackPendingRequest(const unsigned int id, IpcIoPendingRequest *const pending);
     void push(IpcIoPendingRequest *const pending);
     IpcIoPendingRequest *dequeueRequest(const unsigned int requestId);