Postfix for #8429: Segfault when already destroyed callback interface is used; more performance optimization

Author: AlexPeshkoff

src/jrd/extds/ExtDS.cpp

@@ -207,15 +207,15 @@ Connection* Manager::getConnection(thread_db* tdbb, const string& dataSource,
 
 	Provider* prv = getProvider(prvName);
 
-	const bool isCurrent = (prvName == INTERNAL_PROVIDER_NAME) &&
+	const bool isCurrentAtt = (prvName == INTERNAL_PROVIDER_NAME) &&
 		isCurrentAccount(att->att_user, user, pwd, role);
 
 	ClumpletWriter dpb(ClumpletReader::dpbList, MAX_DPB_SIZE);
-	if (!isCurrent)
+	if (!isCurrentAtt)
 		prv->generateDPB(tdbb, dpb, user, pwd, role);
 
 	// look up at connections already bound to current attachment
-	Connection* conn = prv->getBoundConnection(tdbb, dbName, dpb, tra_scope);
+	Connection* conn = prv->getBoundConnection(tdbb, dbName, dpb, tra_scope, isCurrentAtt);
 	if (conn)
 		return conn;
 
@@ -226,7 +226,7 @@ Connection* Manager::getConnection(thread_db* tdbb, const string& dataSource,
 
 	ULONG hash = 0;
 
-	if (!isCurrent)
+	if (!isCurrentAtt)
 	{
 		CryptHash ch(att->att_crypt_callback);
 		hash = DefaultHash<UCHAR>::hash(dbName.c_str(), dbName.length(), MAX_ULONG) +
@@ -255,7 +255,7 @@ Connection* Manager::getConnection(thread_db* tdbb, const string& dataSource,
 	{
 		// finally, create new connection
 		conn = prv->createConnection(tdbb, dbName, dpb, tra_scope);
-		if (!isCurrent)
+		if (!isCurrentAtt)
 			m_connPool->addConnection(tdbb, conn, hash);
 	}
 
@@ -358,7 +358,10 @@ Connection* Provider::createConnection(thread_db* tdbb,
 	const PathName& dbName, ClumpletReader& dpb, TraScope tra_scope)
 {
 	Connection* conn = doCreateConnection();
-	conn->setup(dbName, dpb, tdbb->getAttachment()->att_crypt_callback);
+	conn->setup(dbName, dpb);
+	if (!conn->isCurrent())
+		conn->setCallbackRedirect(tdbb->getAttachment()->att_crypt_callback);
+
 	try
 	{
 		conn->attach(tdbb);
@@ -390,11 +393,13 @@ void Provider::bindConnection(thread_db* tdbb, Connection* conn)
 
 Connection* Provider::getBoundConnection(Jrd::thread_db* tdbb,
 	const Firebird::PathName& dbName, Firebird::ClumpletReader& dpb,
-	TraScope tra_scope)
+	TraScope tra_scope, bool isCurrentAtt)
 {
 	Database* dbb = tdbb->getDatabase();
 	Attachment* att = tdbb->getAttachment();
-	CryptHash ch(att->att_crypt_callback);
+	CryptHash ch;
+	if (!isCurrentAtt)
+		ch.assign(att->att_crypt_callback);
 
 	MutexLockGuard guard(m_mutex, FB_FUNCTION);
 
@@ -590,14 +595,12 @@ Connection::Connection(Provider& prov) :
 {
 }
 
-void Connection::setup(const PathName& dbName, const ClumpletReader& dpb, ICryptKeyCallback* attCallback)
+void Connection::setup(const PathName& dbName, const ClumpletReader& dpb)
 {
 	m_dbName = dbName;
 
 	m_dpb.clear();
 	m_dpb.add(dpb.getBuffer(), dpb.getBufferLength());
-
-	m_cryptCallbackRedir.setRedirect(attCallback);
 }
 
 void Connection::deleteConnection(thread_db* tdbb, Connection* conn)
@@ -2668,7 +2671,7 @@ void CryptHash::assign(ICryptKeyCallback* callback)
 
 bool CryptHash::operator==(const CryptHash& h) const
 {
-	return isValid() && h.isValid() && m_value == h.m_value;
+	return (isValid() == h.isValid()) && (m_value == h.m_value);
 }
 
 

src/jrd/extds/ExtDS.h

@@ -192,7 +192,7 @@ class Provider : public Firebird::GlobalStorage
 	// get available connection already bound to the current attachment
 	Connection* getBoundConnection(Jrd::thread_db* tdbb,
 		const Firebird::PathName& dbName, Firebird::ClumpletReader& dpb,
-		TraScope tra_scope);
+		TraScope tra_scope, bool isCurrent);
 
 	// Connection gets unused, release it into pool or delete it immediately
 	virtual void releaseConnection(Jrd::thread_db* tdbb, Connection& conn, bool inPool = true);
@@ -491,7 +491,12 @@ class Connection : public Firebird::PermanentStorage
 	virtual ~Connection();
 
 	static void deleteConnection(Jrd::thread_db* tdbb, Connection* conn);
-	void setup(const Firebird::PathName& dbName, const Firebird::ClumpletReader& dpb, Firebird::ICryptKeyCallback* attCallback);
+	void setup(const Firebird::PathName& dbName, const Firebird::ClumpletReader& dpb);
+
+	void setCallbackRedirect(Firebird::ICryptKeyCallback* attCallback)
+	{
+		m_cryptCallbackRedir.setRedirect(attCallback);
+	}
 
 	void setBoundAtt(Jrd::Attachment* att) { m_boundAtt = att; }
 

src/jrd/extds/InternalDS.cpp

@@ -136,14 +136,10 @@ void InternalConnection::attach(thread_db* tdbb)
 	// Don't wrap raised errors. This is needed for backward compatibility.
 	setWrapErrors(false);
 
-	if (m_dpb.isEmpty())
-	{
-		m_isCurrent = true;
+	if (isCurrent())
 		m_attachment = attachment->getInterface();
-	}
 	else
 	{
-		m_isCurrent = false;
 		m_dbName = dbb->dbb_database_name.c_str();
 
 		// Avoid change of m_dpb by validatePassword() below
@@ -182,7 +178,7 @@ void InternalConnection::doDetach(thread_db* tdbb)
 	if (!m_attachment->getHandle())
 		return;
 
-	if (m_isCurrent)
+	if (isCurrent())
 	{
 		m_attachment = NULL;
 	}
@@ -221,7 +217,7 @@ bool InternalConnection::cancelExecution(bool /*forced*/)
 	if (!m_attachment->getHandle())
 		return false;
 
-	if (m_isCurrent)
+	if (isCurrent())
 		return true;
 
 	FbLocalStatus status;
@@ -232,9 +228,9 @@ bool InternalConnection::cancelExecution(bool /*forced*/)
 
 bool InternalConnection::resetSession(thread_db* tdbb)
 {
-	fb_assert(!m_isCurrent);
+	fb_assert(isCurrent());
 
-	if (m_isCurrent)
+	if (isCurrent())
 		return true;
 
 	FbLocalStatus status;
@@ -252,13 +248,13 @@ bool InternalConnection::resetSession(thread_db* tdbb)
 // b) is not current connection
 bool InternalConnection::isAvailable(thread_db* tdbb, TraScope /*traScope*/) const
 {
-	return !m_isCurrent ||
-		(m_isCurrent && (tdbb->getAttachment() == m_attachment->getHandle()));
+	return (!isCurrent()) ||
+		(isCurrent() && (tdbb->getAttachment() == m_attachment->getHandle()));
 }
 
 bool InternalConnection::validate(thread_db* tdbb)
 {
-	if (m_isCurrent)
+	if (isCurrent())
 		return true;
 
 	if (!m_attachment)
@@ -272,7 +268,7 @@ bool InternalConnection::validate(thread_db* tdbb)
 
 bool InternalConnection::isSameDatabase(const PathName& dbName, ClumpletReader& dpb, const CryptHash& ch) const
 {
-	if (m_isCurrent)
+	if (isCurrent())
 	{
 		const Attachment* att = m_attachment->getHandle();
 		const MetaString& attUser = att->getUserName();

src/jrd/extds/InternalDS.h

@@ -56,8 +56,7 @@ class InternalConnection : public Connection
 
 	explicit InternalConnection(InternalProvider& prov) :
 	  Connection(prov),
-	  m_attachment(0),
-	  m_isCurrent(false)
+	  m_attachment(0)
 	{}
 
 	virtual ~InternalConnection();
@@ -76,7 +75,7 @@ class InternalConnection : public Connection
 	virtual bool isSameDatabase(const Firebird::PathName& dbName,
 		Firebird::ClumpletReader& dpb, const CryptHash& ch) const override;
 
-	virtual bool isCurrent() const { return m_isCurrent; }
+	virtual bool isCurrent() const { return m_dpb.isEmpty(); }
 
 	Jrd::JAttachment* getJrdAtt() { return m_attachment; }
 
@@ -89,7 +88,6 @@ class InternalConnection : public Connection
 
 	Firebird::AutoPlugin<Jrd::JProvider> m_provider;
 	Firebird::RefPtr<Jrd::JAttachment> m_attachment;
-	bool m_isCurrent;
 };