From 9e9301158034cc1e06283b303f8e1ebc25fff4ec Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Fri, 23 Jun 2017 18:01:51 -0600 Subject: [PATCH] Fixed 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. --- src/ipc/Forwarder.h | 4 +++- src/mgr/Forwarder.cc | 11 ----------- src/mgr/Forwarder.h | 1 - src/tests/stub_libmgr.cc | 1 - 4 files changed, 3 insertions(+), 14 deletions(-) diff --git a/src/ipc/Forwarder.h b/src/ipc/Forwarder.h index 122fc579b8..1f82c6e9be 100644 --- 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 d07ac0c2c8..16f072e925 100644 --- 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 f0f5cf8de6..40bf452e88 100644 --- 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 97e1d2505e..32a21933b8 100644 --- 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) -- 2.47.2