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 122fc579b813533013c3f4df8e82f3ee8918be34..a63b2ef5706ea34373b57642fa28c2a0b754dfd1 100644 (file)
--- a/src/ipc/Forwarder.h
+++ b/src/ipc/Forwarder.h
@@ -49,12 +49,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 d07ac0c2c866b2a7ac580226a5eafed78983f7f1..16f072e925bc66743440b924f35ebb47697d9b41 100644 (file)
--- a/src/mgr/Forwarder.cc
+++ b/src/mgr/Forwarder.cc
@@ -102,17 +102,6 @@ Mgr::Forwarder::noteCommClosed(const CommCloseCbParams &)
     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 f0f5cf8de6cdc0f2aec9cf762babe3dedefec533..40bf452e88300fbb63a4c018d5309c913c9bdfd4 100644 (file)
--- a/src/mgr/Forwarder.h
+++ b/src/mgr/Forwarder.h
@@ -42,7 +42,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)