From bfa196c004872e4d2554b94ab897cdfc14b32fad Mon Sep 17 00:00:00 2001
From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Sat, 15 Apr 2017 23:17:24 +0200
Subject: [PATCH] auth: Don't reconnect to a SQL backend if we are in a
 transaction

---
 modules/gmysqlbackend/gmysqlbackend.cc |  7 -------
 modules/gmysqlbackend/gmysqlbackend.hh |  3 +--
 modules/gpgsqlbackend/gpgsqlbackend.cc | 16 ++++++++++++++++
 modules/gpgsqlbackend/gpgsqlbackend.hh |  3 +++
 modules/gpgsqlbackend/spgsql.hh        |  2 +-
 pdns/backends/gsql/gsqlbackend.cc      |  6 ++++++
 pdns/backends/gsql/gsqlbackend.hh      | 16 ++++++++++------
 7 files changed, 37 insertions(+), 16 deletions(-)

diff --git a/modules/gmysqlbackend/gmysqlbackend.cc b/modules/gmysqlbackend/gmysqlbackend.cc
index cb84b21a6..e0b901236 100644
--- a/modules/gmysqlbackend/gmysqlbackend.cc
+++ b/modules/gmysqlbackend/gmysqlbackend.cc
@@ -62,13 +62,6 @@ void gMySQLBackend::reconnect()
                    getArgAsNum("timeout")));
 }
 
-void gMySQLBackend::reconnectIfNeeded()
-{
-  if (!isConnectionUsable()) {
-    reconnect();
-  }
-}
-
 class gMySQLFactory : public BackendFactory
 {
 public:
diff --git a/modules/gmysqlbackend/gmysqlbackend.hh b/modules/gmysqlbackend/gmysqlbackend.hh
index be6acfea2..67f6928a3 100644
--- a/modules/gmysqlbackend/gmysqlbackend.hh
+++ b/modules/gmysqlbackend/gmysqlbackend.hh
@@ -35,8 +35,7 @@ class gMySQLBackend : public GSQLBackend
 public:
   gMySQLBackend(const string &mode, const string &suffix); //!< Makes our connection to the database. Throws an exception if it fails.
 protected:
-  void reconnectIfNeeded() override;
-  void reconnect();
+  void reconnect() override;
 };
 
 #endif /* PDNS_GMYSQLBACKEND_HH */
diff --git a/modules/gpgsqlbackend/gpgsqlbackend.cc b/modules/gpgsqlbackend/gpgsqlbackend.cc
index f41ccd0b2..ab43d80b5 100644
--- a/modules/gpgsqlbackend/gpgsqlbackend.cc
+++ b/modules/gpgsqlbackend/gpgsqlbackend.cc
@@ -54,6 +54,22 @@ gPgSQLBackend::gPgSQLBackend(const string &mode, const string &suffix)  : GSQLBa
   L<<Logger::Info<<mode<<" Connection successful. Connected to database '"<<getArg("dbname")<<"' on '"<<getArg("host")<<"'."<<endl;
 }
 
+void gPgSQLBackend::reconnect()
+{
+  if (d_db) {
+    d_db->reconnect();
+  }
+}
+
+bool gPgSQLBackend::inTransaction()
+{
+  const auto* db = dynamic_cast<SPgSQL*>(d_db);
+  if (db) {
+    return db->in_trx();
+  }
+  return false;
+}
+
 class gPgSQLFactory : public BackendFactory
 {
 public:
diff --git a/modules/gpgsqlbackend/gpgsqlbackend.hh b/modules/gpgsqlbackend/gpgsqlbackend.hh
index a93f471b6..b1bef1da9 100644
--- a/modules/gpgsqlbackend/gpgsqlbackend.hh
+++ b/modules/gpgsqlbackend/gpgsqlbackend.hh
@@ -34,6 +34,9 @@ class gPgSQLBackend : public GSQLBackend
 {
 public:
   gPgSQLBackend(const string &mode, const string &suffix); //!< Makes our connection to the database. Throws an exception if it fails.
+protected:
+  void reconnect() override;
+  bool inTransaction() override;
 };
 
 #endif /* PDNS_GPGSQLBACKEND_HH */
diff --git a/modules/gpgsqlbackend/spgsql.hh b/modules/gpgsqlbackend/spgsql.hh
index bf39b3ab5..6654e4f87 100644
--- a/modules/gpgsqlbackend/spgsql.hh
+++ b/modules/gpgsqlbackend/spgsql.hh
@@ -47,7 +47,7 @@ public:
   void reconnect() override;
 
   PGconn* db() { return d_db; }
-  bool in_trx() { return d_in_trx; }
+  bool in_trx() const { return d_in_trx; }
 
 private:
   PGconn* d_db;
diff --git a/pdns/backends/gsql/gsqlbackend.cc b/pdns/backends/gsql/gsqlbackend.cc
index 53b2faf6a..efc9b5def 100644
--- a/pdns/backends/gsql/gsqlbackend.cc
+++ b/pdns/backends/gsql/gsqlbackend.cc
@@ -1464,6 +1464,7 @@ bool GSQLBackend::startTransaction(const DNSName &domain, int domain_id)
     reconnectIfNeeded();
 
     d_db->startTransaction();
+    d_inTransaction = true;
     if(domain_id >= 0) {
       d_DeleteZoneQuery_stmt->
         bind("domain_id", domain_id)->
@@ -1472,6 +1473,7 @@ bool GSQLBackend::startTransaction(const DNSName &domain, int domain_id)
     }
   }
   catch (SSqlException &e) {
+    d_inTransaction = false;
     throw PDNSException("Database failed to start transaction: "+e.txtReason());
   }
 
@@ -1482,8 +1484,10 @@ bool GSQLBackend::commitTransaction()
 {
   try {
     d_db->commit();
+    d_inTransaction = false;
   }
   catch (SSqlException &e) {
+    d_inTransaction = false;
     throw PDNSException("Database failed to commit transaction: "+e.txtReason());
   }
   return true;
@@ -1493,8 +1497,10 @@ bool GSQLBackend::abortTransaction()
 {
   try {
     d_db->rollback();
+    d_inTransaction = false;
   }
   catch(SSqlException &e) {
+    d_inTransaction = false;
     throw PDNSException("Database failed to abort transaction: "+string(e.txtReason()));
   }
   return true;
diff --git a/pdns/backends/gsql/gsqlbackend.hh b/pdns/backends/gsql/gsqlbackend.hh
index 8fdb2393c..4f8543088 100644
--- a/pdns/backends/gsql/gsqlbackend.hh
+++ b/pdns/backends/gsql/gsqlbackend.hh
@@ -250,21 +250,23 @@ protected:
     }
     return false;
   }
-  virtual void reconnectIfNeeded()
+  void reconnectIfNeeded()
   {
-    if (isConnectionUsable()) {
+    if (isConnectionUsable() || inTransaction()) {
       return;
     }
 
-    if (d_db) {
-      d_db->reconnect();
-    }
+    reconnect();
+  }
+  virtual void reconnect() { }
+  virtual bool inTransaction()
+  {
+    return d_inTransaction;
   }
 
 private:
   string d_query_name;
   DNSName d_qname;
-  SSql *d_db;
   SSqlStatement::result_t d_result;
 
   string d_NoIdQuery;
@@ -402,7 +404,9 @@ private:
   SSqlStatement* d_SearchCommentsQuery_stmt;
 
 protected:
+  SSql *d_db{nullptr};
   bool d_dnssecQueries;
+  bool d_inTransaction{false};
 };
 
 #endif /* PDNS_GSQLBACKEND_HH */
-- 
2.49.0