Fix mgr query handoff from the original recipient to Coordinator.

This bug has already been fixed once, in trunk r11164.1.61, but that fix
was accidentally undone shortly after, during significant cross-branch
merging activity combined with the Forwarder class split. The final
merge importing the associated code (trunk r11730) was buggy.

The bug (explained in r11164.1.61) leads to a race condition between

* Store notifying Server classes about the entry completion (which might
  trigger a bogus error message sent to the cache manager client while
  Coordinator sends its own valid response on the same connection!) and

* post-cleanup() connection closure handlers of Server classes silently
  closing everything (and leaving Coordinator the only responding
  process on that shared connection).

The bug probably was not noticed for so long because, evidently, the
latter actions tend to win in the current code.

diff --git a/src/ipc/Forwarder.h b/src/ipc/Forwarder.h
index 8cdd72ac449dba2f0408b0f2c7b3f869f916e6fa..20653cdf4f1c671a9774ad81979553a19f8a3426 100644 (file)
--- a/src/ipc/Forwarder.h
+++ b/src/ipc/Forwarder.h
@@ -47,12 +47,14 @@ protected:
     virtual void handleError();
     virtual void handleTimeout();
     virtual void handleException(const std::exception& e);
-    virtual void handleRemoteAck();
 
 private:
     static void RequestTimedOut(void* param);
     void requestTimedOut();
     void removeTimeoutEvent();
+
+    void handleRemoteAck();
+
     static AsyncCall::Pointer DequeueRequest(unsigned int requestId);
 
 protected:

diff --git a/src/mgr/Forwarder.cc b/src/mgr/Forwarder.cc
index 2038b108d747925405816536c0e4840524d06bbb..c890428e92ae93aa65749ec62c1837c194501551 100644 (file)
--- a/src/mgr/Forwarder.cc
+++ b/src/mgr/Forwarder.cc
@@ -102,17 +102,6 @@ Mgr::Forwarder::noteCommClosed(const CommCloseCbParams& params)
     mustStop("commClosed");
 }
 
-/// called when Coordinator starts processing the request
-void
-Mgr::Forwarder::handleRemoteAck()
-{
-    Ipc::Forwarder::handleRemoteAck();
-
-    Must(entry != NULL);
-    EBIT_CLR(entry->flags, ENTRY_FWD_HDR_WAIT);
-    entry->complete();
-}
-
 /// send error page
 void
 Mgr::Forwarder::sendError(ErrorState *error)

diff --git a/src/mgr/Forwarder.h b/src/mgr/Forwarder.h
index 3224a999b1aa1d7aa3d736c22c1f8b407c1fe272..a6b3c9dac42512eef875b0d9f30fc9c92d6b19db 100644 (file)
--- a/src/mgr/Forwarder.h
+++ b/src/mgr/Forwarder.h
@@ -40,7 +40,6 @@ protected:
     virtual void handleError();
     virtual void handleTimeout();
     virtual void handleException(const std::exception& e);
-    virtual void handleRemoteAck();
 
 private:
     void noteCommClosed(const CommCloseCbParams& params);

diff --git a/src/tests/stub_libmgr.cc b/src/tests/stub_libmgr.cc
index 97e1d2505e2f1306666c273df1f17fc38e6458bf..32a21933b8590a5c9513be9ac090791ea29acc04 100644 (file)
--- a/src/tests/stub_libmgr.cc
+++ b/src/tests/stub_libmgr.cc
@@ -100,7 +100,6 @@ void Mgr::Forwarder::cleanup() STUB
 void Mgr::Forwarder::handleError() STUB
 void Mgr::Forwarder::handleTimeout() STUB
 void Mgr::Forwarder::handleException(const std::exception& e) STUB
-void Mgr::Forwarder::handleRemoteAck() STUB
 
 #include "mgr/FunAction.h"
 Mgr::Action::Pointer Mgr::FunAction::Create(const CommandPointer &cmd, OBJH *aHandler) STUB_RETVAL(dummyAction)