From c67eb9c49756bb4bea07aba58146987bf8cf2d75 Mon Sep 17 00:00:00 2001 From: hvlad Date: Mon, 25 Dec 2017 17:09:34 +0200 Subject: [PATCH] It should fix bug CORE-5685 : Sometime it is impossible to cancel\kill connection executing external query. Also, implement fb_cancel_abort option at engine and unify handling of network errors a bit. --- src/common/utils_proto.h | 8 ++++++++ src/jrd/Attachment.cpp | 4 ++-- src/jrd/extds/ExtDS.cpp | 23 ++++++++++++++++------- src/jrd/extds/ExtDS.h | 3 ++- src/jrd/extds/InternalDS.cpp | 2 +- src/jrd/extds/InternalDS.h | 2 +- src/jrd/extds/IscDS.cpp | 20 ++++++++------------ src/jrd/extds/IscDS.h | 2 +- src/jrd/jrd.cpp | 8 ++++++++ src/remote/client/interface.cpp | 3 ++- src/yvalve/why.cpp | 5 ++++- 11 files changed, 53 insertions(+), 27 deletions(-) diff --git a/src/common/utils_proto.h b/src/common/utils_proto.h index 6acfa9e63d5..13ccc0025a0 100644 --- a/src/common/utils_proto.h +++ b/src/common/utils_proto.h @@ -191,6 +191,14 @@ namespace fb_utils // Check does vector contain particular code or not bool containsErrorCode(const ISC_STATUS* v, ISC_STATUS code); + bool inline isNetworkError(ISC_STATUS code) + { + return code == isc_network_error || + code == isc_net_write_err || + code == isc_net_read_err || + code == isc_lost_db_connection; + } + // Uppercase/strip string according to login rules const char* dpbItemUpper(const char* s, FB_SIZE_T l, Firebird::string& buf); diff --git a/src/jrd/Attachment.cpp b/src/jrd/Attachment.cpp index 96c570ddb7e..10e02ccffc2 100644 --- a/src/jrd/Attachment.cpp +++ b/src/jrd/Attachment.cpp @@ -365,7 +365,7 @@ void Jrd::Attachment::signalCancel() att_flags |= ATT_cancel_raise; if (att_ext_connection && att_ext_connection->isConnected()) - att_ext_connection->cancelExecution(); + att_ext_connection->cancelExecution(false); LCK_cancel_wait(this); } @@ -376,7 +376,7 @@ void Jrd::Attachment::signalShutdown() att_flags |= ATT_shutdown; if (att_ext_connection && att_ext_connection->isConnected()) - att_ext_connection->cancelExecution(); + att_ext_connection->cancelExecution(true); LCK_cancel_wait(this); } diff --git a/src/jrd/extds/ExtDS.cpp b/src/jrd/extds/ExtDS.cpp index d255e8cdc2d..7e275dcb761 100644 --- a/src/jrd/extds/ExtDS.cpp +++ b/src/jrd/extds/ExtDS.cpp @@ -273,7 +273,7 @@ void Provider::cancelConnections() Connection** end = m_connections.end(); for (; ptr < end; ptr++) { - (*ptr)->cancelExecution(); + (*ptr)->cancelExecution(true); } } @@ -1618,8 +1618,13 @@ void EngineCallbackGuard::init(thread_db* tdbb, Connection& conn, const char* fr if (attachment) { m_saveConnection = attachment->att_ext_connection; - attachment->att_ext_connection = &conn; - attachment->getStable()->getMutex()->leave(); + m_stable = attachment->getStable(); + m_stable->getMutex()->leave(); + + MutexLockGuard guardAsync(*m_stable->getMutex(true, true), FB_FUNCTION); + MutexLockGuard guardMain(*m_stable->getMutex(), FB_FUNCTION); + if (m_stable->getHandle() == attachment) + attachment->att_ext_connection = &conn; } } @@ -1637,11 +1642,15 @@ EngineCallbackGuard::~EngineCallbackGuard() if (m_tdbb) { Jrd::Attachment* attachment = m_tdbb->getAttachment(); - - if (attachment) + if (attachment && m_stable.hasData()) { - attachment->getStable()->getMutex()->enter(FB_FUNCTION); - attachment->att_ext_connection = m_saveConnection; + MutexLockGuard guardAsync(*m_stable->getMutex(true, true), FB_FUNCTION); + m_stable->getMutex()->enter(FB_FUNCTION); + + if (m_stable->getHandle() == attachment) + attachment->att_ext_connection = m_saveConnection; + else + m_stable->getMutex()->leave(); } jrd_tra* transaction = m_tdbb->getTransaction(); diff --git a/src/jrd/extds/ExtDS.h b/src/jrd/extds/ExtDS.h index 6d5be00a562..0e6db116051 100644 --- a/src/jrd/extds/ExtDS.h +++ b/src/jrd/extds/ExtDS.h @@ -160,7 +160,7 @@ class Connection : public Firebird::PermanentStorage const Firebird::string& role) = 0; virtual void detach(Jrd::thread_db* tdbb); - virtual bool cancelExecution() = 0; + virtual bool cancelExecution(bool forced) = 0; int getSqlDialect() const { return m_sqlDialect; } @@ -468,6 +468,7 @@ class EngineCallbackGuard void init(Jrd::thread_db* tdbb, Connection& conn, const char* from); Jrd::thread_db* m_tdbb; + Firebird::RefPtr m_stable; Firebird::Mutex* m_mutex; Connection* m_saveConnection; }; diff --git a/src/jrd/extds/InternalDS.cpp b/src/jrd/extds/InternalDS.cpp index b71708cb374..476b9fb371a 100644 --- a/src/jrd/extds/InternalDS.cpp +++ b/src/jrd/extds/InternalDS.cpp @@ -218,7 +218,7 @@ void InternalConnection::doDetach(thread_db* tdbb) fb_assert(!m_attachment); } -bool InternalConnection::cancelExecution() +bool InternalConnection::cancelExecution(bool /*forced*/) { if (!m_attachment->getHandle()) return false; diff --git a/src/jrd/extds/InternalDS.h b/src/jrd/extds/InternalDS.h index 94d9884f923..383f13c5dd7 100644 --- a/src/jrd/extds/InternalDS.h +++ b/src/jrd/extds/InternalDS.h @@ -67,7 +67,7 @@ class InternalConnection : public Connection const Firebird::string& user, const Firebird::string& pwd, const Firebird::string& role); - virtual bool cancelExecution(); + virtual bool cancelExecution(bool forced); virtual bool isAvailable(Jrd::thread_db* tdbb, TraScope traScope) const; diff --git a/src/jrd/extds/IscDS.cpp b/src/jrd/extds/IscDS.cpp index a3597526a5f..42dec468879 100644 --- a/src/jrd/extds/IscDS.cpp +++ b/src/jrd/extds/IscDS.cpp @@ -221,15 +221,18 @@ void IscConnection::doDetach(thread_db* tdbb) } } -bool IscConnection::cancelExecution() +bool IscConnection::cancelExecution(bool forced) { FbLocalStatus status; if (m_handle) { - m_iscProvider.fb_cancel_operation(&status, &m_handle, fb_cancel_raise); + m_iscProvider.fb_cancel_operation(&status, &m_handle, + forced ? fb_cancel_abort : fb_cancel_raise); - if (m_handle && (status->getErrors()[1] == isc_wish_list)) + if (!forced && m_handle && + (status->getState() & IStatus::STATE_ERRORS) && + (status->getErrors()[1] != isc_bad_db_handle)) { status->init(); m_iscProvider.fb_cancel_operation(&status, &m_handle, fb_cancel_abort); @@ -1608,15 +1611,8 @@ void FBProvider::loadAPI() static bool isConnectionBrokenError(FbStatusVector* status) { - switch (status->getErrors()[1]) - { - case isc_att_shutdown: - case isc_network_error: - case isc_net_read_err: - case isc_net_write_err: - return true; - } - return false; + ISC_STATUS code = status->getErrors()[1]; + return (fb_utils::isNetworkError(code) || code == isc_att_shutdown); } diff --git a/src/jrd/extds/IscDS.h b/src/jrd/extds/IscDS.h index b472794f395..0638b8df276 100644 --- a/src/jrd/extds/IscDS.h +++ b/src/jrd/extds/IscDS.h @@ -517,7 +517,7 @@ class IscConnection : public Connection const Firebird::string& user, const Firebird::string& pwd, const Firebird::string& role); - virtual bool cancelExecution(); + virtual bool cancelExecution(bool forced); virtual bool isAvailable(Jrd::thread_db* tdbb, TraScope traScope) const; diff --git a/src/jrd/jrd.cpp b/src/jrd/jrd.cpp index c052a890de9..c014861efc2 100644 --- a/src/jrd/jrd.cpp +++ b/src/jrd/jrd.cpp @@ -2874,6 +2874,9 @@ void JAttachment::detach(CheckStatusWrapper* user_status) * Close down a database. * **************************************/ + if (!att->getHandle()) + return; // already detached + RefDeb(DEB_RLS_JATT, "JAttachment::detach"); freeEngineData(user_status, false); } @@ -8098,6 +8101,11 @@ void JRD_cancel_operation(thread_db* /*tdbb*/, Jrd::Attachment* attachment, int attachment->signalCancel(); break; + case fb_cancel_abort: + if (!(attachment->att_flags & ATT_shutdown)) + attachment->signalShutdown(); + break; + default: fb_assert(false); } diff --git a/src/remote/client/interface.cpp b/src/remote/client/interface.cpp index 602c19765a9..6b2e0831dfa 100644 --- a/src/remote/client/interface.cpp +++ b/src/remote/client/interface.cpp @@ -1613,7 +1613,8 @@ void Attachment::freeClientData(CheckStatusWrapper* status, bool force) // telling the user that an unrecoverable network error occurred and that // if there was any uncommitted work, its gone...... Oh well.... ex.stuffException(status); - if ((status->getErrors()[1] != isc_network_error) && (!force)) + + if (!fb_utils::isNetworkError(status->getErrors()[1]) && (!force)) { return; } diff --git a/src/yvalve/why.cpp b/src/yvalve/why.cpp index 7e942c6434b..f3d32029655 100644 --- a/src/yvalve/why.cpp +++ b/src/yvalve/why.cpp @@ -1325,7 +1325,7 @@ static void badHandle(ISC_STATUS code) static bool isNetworkError(const IStatus* status) { ISC_STATUS code = status->getErrors()[1]; - return code == isc_network_error || code == isc_net_write_err || code == isc_net_read_err; + return fb_utils::isNetworkError(code); } static void nullCheck(const FB_API_HANDLE* ptr, ISC_STATUS code) @@ -5457,6 +5457,9 @@ void YAttachment::detach(CheckStatusWrapper* status) if (entry.next()) entry.next()->detach(status); + if (isNetworkError(status)) + status->init(); + if (!(status->getState() & Firebird::IStatus::STATE_ERRORS)) destroy(DF_RELEASE); }