Backport fix for bug CORE-2731 : Recursive EXECUTE STATEMENT works wrong

src/include/consts_pub.h

@@ -108,6 +108,7 @@
 #define isc_dpb_trusted_role			  75
 #define isc_dpb_org_filename			  76
 #define isc_dpb_utf8_filename			  77
+#define isc_dpb_ext_call_depth			  78
 
 /**************************************************/
 /* clumplet tags used inside isc_dpb_address_path */

src/jrd/execute_statement.h

@@ -34,8 +34,6 @@
 #include "../jrd/exe.h"
 #include "../jrd/ibase.h"
 
-const int MAX_CALLBACKS	= 50;
-
 namespace Jrd {
 
 

src/jrd/extds/ExtDS.cpp

@@ -185,6 +185,11 @@ Provider::~Provider()
 Connection* Provider::getConnection(thread_db* tdbb, const string& dbName,
 	const string& user, const string& pwd, const string& role, TraScope tra_scope)
 {
+	const Attachment* attachment = tdbb->getAttachment();
+
+	if (attachment->att_ext_call_depth >= MAX_CALLBACKS)
+		ERR_post(Arg::Gds(isc_exec_sql_max_call_exceeded));
+
 	{ // m_mutex scope
 		Database::CheckoutLockGuard guard(tdbb->getDatabase(), m_mutex);
 
@@ -194,7 +199,8 @@ Connection* Provider::getConnection(thread_db* tdbb, const string& dbName,
 		for (; conn_ptr < end; conn_ptr++)
 		{
 			Connection* conn = *conn_ptr;
-			if (conn->isSameDatabase(tdbb, dbName, user, pwd, role) &&
+			if (conn->m_boundAtt == attachment &&
+				conn->isSameDatabase(tdbb, dbName, user, pwd, role) &&
 				conn->isAvailable(tdbb, tra_scope))
 			{
 				return conn;
@@ -206,6 +212,7 @@ Connection* Provider::getConnection(thread_db* tdbb, const string& dbName,
 	try
 	{
 		conn->attach(tdbb, dbName, user, pwd, role);
+		conn->m_boundAtt = attachment;
 	}
 	catch (...)
 	{
@@ -228,6 +235,8 @@ void Provider::releaseConnection(thread_db* tdbb, Connection& conn, bool /*inPoo
 	{ // m_mutex scope
 		Database::CheckoutLockGuard guard(tdbb->getDatabase(), m_mutex);
 
+		conn.m_boundAtt = NULL;
+
 		size_t pos;
 		if (!m_connections.find(&conn, pos))
 		{
@@ -282,6 +291,7 @@ Connection::Connection(Provider& prov) :
 	m_transactions(getPool()),
 	m_statements(getPool()),
 	m_freeStatements(NULL),
+	m_boundAtt(NULL),
 	m_used_stmts(0),
 	m_free_stmts(0),
 	m_deleting(false),
@@ -308,8 +318,11 @@ void Connection::generateDPB(thread_db* tdbb, ClumpletWriter& dpb,
 {
 	dpb.reset(isc_dpb_version1);
 
-	const string& attUser = tdbb->getAttachment()->att_user->usr_user_name;
-	const string& attRole = tdbb->getAttachment()->att_user->usr_sql_role_name;
+	const Attachment *attachment = tdbb->getAttachment();
+	dpb.insertInt(isc_dpb_ext_call_depth, attachment->att_ext_call_depth + 1);
+
+	const string& attUser = attachment->att_user->usr_user_name;
+	const string& attRole = attachment->att_user->usr_sql_role_name;
 
 	if ((m_provider.getFlags() & prvTrustedAuth) &&
 		(user.isEmpty() || user == attUser) && pwd.isEmpty() &&
@@ -331,7 +344,7 @@ void Connection::generateDPB(thread_db* tdbb, ClumpletWriter& dpb,
 		}
 	}
 
-	CharSet* const cs = INTL_charset_lookup(tdbb, tdbb->getAttachment()->att_charset);
+	CharSet* const cs = INTL_charset_lookup(tdbb, attachment->att_charset);
 	if (cs) {
 		dpb.insertString(isc_dpb_lc_ctype, cs->getName());
 	}
@@ -1470,9 +1483,9 @@ void Statement::raise(ISC_STATUS* status, thread_db* tdbb, const char* sWhere,
 	}
 
 	// Execute statement error at @1 :\n@2Statement : @3\nData source : @4
-	ERR_post(Arg::Gds(isc_eds_statement) << Arg::Str(sWhere) <<
+ 	ERR_post(Arg::Gds(isc_eds_statement) << Arg::Str(sWhere) <<
 											Arg::Str(rem_err) <<
-											Arg::Str(sQuery ? sQuery->substr(0, 255) : m_sql.substr(0, 255)) <<
+ 											Arg::Str(sQuery ? sQuery->substr(0, 255) : m_sql.substr(0, 255)) <<
 											Arg::Str(m_connection.getDataSourceName()));
 }
 
@@ -1521,17 +1534,24 @@ void EngineCallbackGuard::init(thread_db* tdbb, Connection& conn)
 {
 	m_tdbb = tdbb;
 	m_mutex = conn.isConnected() ? &conn.m_mutex : &conn.m_provider.m_mutex;
+	m_saveConnection = NULL;
 
 	if (m_tdbb)
 	{
-		if (m_tdbb->getTransaction()) {
-			m_tdbb->getTransaction()->tra_callback_count++;
+		jrd_tra *transaction = m_tdbb->getTransaction();
+		if (transaction) 
+		{
+			if (transaction->tra_callback_count >= MAX_CALLBACKS)
+				ERR_post(Arg::Gds(isc_exec_sql_max_call_exceeded));
+
+			transaction->tra_callback_count++;
 		}
 
-		if (m_tdbb->getAttachment())
+		Attachment *attachment = m_tdbb->getAttachment();
+		if (attachment)
 		{
-			fb_assert(!m_tdbb->getAttachment()->att_ext_connection);
-			m_tdbb->getAttachment()->att_ext_connection = &conn;
+			m_saveConnection = attachment->att_ext_connection;
+			attachment->att_ext_connection = &conn;
 		}
 
 		m_tdbb->getDatabase()->dbb_sync->unlock();
@@ -1552,12 +1572,14 @@ EngineCallbackGuard::~EngineCallbackGuard()
 	{
 		m_tdbb->getDatabase()->dbb_sync->lock();
 
-		if (m_tdbb->getTransaction()) {
-			m_tdbb->getTransaction()->tra_callback_count--;
+		jrd_tra *transaction = m_tdbb->getTransaction();
+		if (transaction) {
+			transaction->tra_callback_count--;
 		}
 
-		if (m_tdbb->getAttachment()) {
-			m_tdbb->getAttachment()->att_ext_connection = NULL;
+		Attachment *attachment = m_tdbb->getAttachment();
+		if (attachment) {
+			attachment->att_ext_connection = m_saveConnection;
 		}
 	}
 }

src/jrd/extds/ExtDS.h

@@ -147,6 +147,7 @@ class Connection : public Firebird::PermanentStorage
 {
 protected:
 	friend class EngineCallbackGuard;
+	friend class Provider;
 
 	explicit Connection(Provider& prov);
 	virtual ~Connection();
@@ -228,6 +229,8 @@ class Connection : public Firebird::PermanentStorage
 	Firebird::Array<Statement*> m_statements;
 	Statement* m_freeStatements;
 
+	const Jrd::Attachment* m_boundAtt;
+
 	static const int MAX_CACHED_STMTS = 16;
 	int	m_used_stmts;
 	int	m_free_stmts;
@@ -454,6 +457,7 @@ class EngineCallbackGuard
 
 	Jrd::thread_db* m_tdbb;
 	Firebird::Mutex* m_mutex;
+	Connection* m_saveConnection;
 };
 
 } // namespace EDS

src/jrd/extds/InternalDS.cpp

@@ -83,7 +83,7 @@ void InternalProvider::getRemoteError(ISC_STATUS* status, string& err) const
 {
 	err = "";
 
-	char buff[512];
+	char buff[1024];
 	const ISC_STATUS* p = status;
 	const ISC_STATUS* end = status + ISC_STATUS_LENGTH;
 
@@ -209,10 +209,9 @@ bool InternalConnection::isSameDatabase(thread_db* tdbb, const Firebird::string&
 		const Firebird::string& user, const Firebird::string& pwd,
 		const Firebird::string& role) const
 {
-	const UserId *attUser = m_attachment->att_user;
-
 	if (m_isCurrent)
 	{
+		const UserId* attUser = m_attachment->att_user;
 		return ((user.isEmpty() || user == attUser->usr_user_name) &&
 				pwd.isEmpty() &&
 				(role.isEmpty() || role == attUser->usr_sql_role_name));

src/jrd/extds/IscDS.cpp

@@ -70,7 +70,13 @@ void IscProvider::getRemoteError(ISC_STATUS* status, string& err) const
 {
 	err = "";
 
-	char buff[512];
+	// We can't use safe fb_interpet here as we have no idea what implementation
+	// of ISC API is used by current provider. We can test for existance of
+	// fb_interpet and use it if present, but i don't want to complicate code.
+	// So, buffer should be big enough to please old isc_interprete.
+	// Probably in next version we should use fb_interpet only.
+
+	char buff[1024];
 	ISC_STATUS* p = status;
 	const ISC_STATUS* const end = status + ISC_STATUS_LENGTH;
 

src/jrd/jrd.cpp

@@ -431,6 +431,7 @@ class DatabaseOptions
 	bool	dpb_gbak_attach;
 	bool	dpb_trusted_role;
 	bool	dpb_utf8_filename;
+	ULONG	dpb_ext_call_depth;
 	ULONG	dpb_flags;			// to OR'd with dbb_flags
 
 	// here begin compound objects
@@ -932,6 +933,7 @@ ISC_STATUS GDS_ATTACH_DATABASE(ISC_STATUS* user_status,
 	attachment->att_remote_pid = options.dpb_remote_pid;
 	attachment->att_remote_process = options.dpb_remote_process;
 	attachment->att_next = dbb->dbb_attachments;
+	attachment->att_ext_call_depth = options.dpb_ext_call_depth;
 
 	dbb->dbb_attachments = attachment;
 	dbb->dbb_flags &= ~DBB_being_opened;
@@ -1936,6 +1938,7 @@ ISC_STATUS GDS_CREATE_DATABASE(ISC_STATUS* user_status,
 	attachment->att_remote_address = options.dpb_remote_address;
 	attachment->att_remote_pid = options.dpb_remote_pid;
 	attachment->att_remote_process = options.dpb_remote_process;
+	attachment->att_ext_call_depth = options.dpb_ext_call_depth;
 	attachment->att_next = dbb->dbb_attachments;
 
 	dbb->dbb_attachments = attachment;
@@ -4753,6 +4756,12 @@ void DatabaseOptions::get(const UCHAR* dpb, USHORT dpb_length, bool& invalid_cli
 			getPath(rdr, dpb_org_filename);
 			break;
 
+		case isc_dpb_ext_call_depth:
+			dpb_ext_call_depth = (ULONG) rdr.getInt();
+			if (dpb_ext_call_depth >= MAX_CALLBACKS)
+				ERR_post(Arg::Gds(isc_exec_sql_max_call_exceeded));
+			break;
+
 		default:
 			break;
 		}
@@ -5200,6 +5209,7 @@ Attachment::Attachment(MemoryPool* pool, Database* dbb)
 	att_dsql_cache(*pool),
 	att_udf_pointers(*pool),
 	att_ext_connection(NULL),
+	att_ext_call_depth(0),
 	att_trace_manager(FB_NEW(*att_pool) TraceManager(this))
 {
 	att_mutex.enter();

src/jrd/jrd.h

@@ -105,6 +105,7 @@ namespace Jrd {
 
 const int QUANTUM			= 100;	// Default quantum
 const int SWEEP_QUANTUM		= 10;	// Make sweeps less disruptive
+const int MAX_CALLBACKS		= 50;
 
 // fwd. decl.
 class thread_db;
@@ -331,6 +332,7 @@ class Attachment : public pool_alloc<type_att>, public Firebird::PublicHandle
 	Firebird::Mutex att_mutex;				// attachment mutex
 
 	EDS::Connection* att_ext_connection;	// external connection executed by this attachment
+	ULONG att_ext_call_depth;				// external connection call depth, 0 for user attachment
 	TraceManager* att_trace_manager;		// Trace API manager
 
 	bool locksmith() const;