It should fix bug CORE-5685 : Sometime it is impossible to cancel\kil…

…l 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

@@ -194,6 +194,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);
 

src/jrd/Attachment.cpp

@@ -369,7 +369,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);
 }
@@ -382,7 +382,7 @@ void Jrd::Attachment::signalShutdown(ISC_STATUS code)
 		getStable()->setShutError(code);
 
 	if (att_ext_connection && att_ext_connection->isConnected())
-		att_ext_connection->cancelExecution();
+		att_ext_connection->cancelExecution(true);
 
 	LCK_cancel_wait(this);
 }

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);
 	}
 }
 
@@ -1623,8 +1623,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;
 		}
 	}
 
@@ -1642,11 +1647,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();

src/jrd/extds/ExtDS.h

@@ -160,7 +160,7 @@ class Connection : public Firebird::PermanentStorage
 		const Firebird::MetaName& 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; }
 
@@ -470,6 +470,7 @@ class EngineCallbackGuard
 	void init(Jrd::thread_db* tdbb, Connection& conn, const char* from);
 
 	Jrd::thread_db* m_tdbb;
+	Firebird::RefPtr<Jrd::StableAttachmentPart> m_stable;
 	Firebird::Mutex* m_mutex;
 	Connection* m_saveConnection;
 };

src/jrd/extds/InternalDS.cpp

@@ -219,7 +219,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;

src/jrd/extds/InternalDS.h

@@ -67,7 +67,7 @@ class InternalConnection : public Connection
 		const Firebird::MetaName& user, const Firebird::string& pwd,
 		const Firebird::MetaName& role);
 
-	virtual bool cancelExecution();
+	virtual bool cancelExecution(bool forced);
 
 	virtual bool isAvailable(Jrd::thread_db* tdbb, TraScope traScope) const;
 

src/jrd/extds/IscDS.cpp

@@ -220,15 +220,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);
@@ -1638,15 +1641,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);
 }
 
 

src/jrd/extds/IscDS.h

@@ -521,7 +521,7 @@ class IscConnection : public Connection
 		const Firebird::MetaName& user, const Firebird::string& pwd,
 		const Firebird::MetaName& role);
 
-	virtual bool cancelExecution();
+	virtual bool cancelExecution(bool forced);
 
 	virtual bool isAvailable(Jrd::thread_db* tdbb, TraScope traScope) const;
 

src/jrd/jrd.cpp

@@ -2924,6 +2924,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);
 }
@@ -8839,6 +8842,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(isc_att_shut_killed);
+		break;
+
 	default:
 		fb_assert(false);
 	}

src/remote/client/interface.cpp

@@ -1866,7 +1866,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;
 			}

src/yvalve/why.cpp

@@ -1349,7 +1349,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)
@@ -5767,6 +5767,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);
 	}