[#2978] Check that mysql pointer is non-null

This is a workaround for the libmysqlclient that dereferences mysql ptr
in the MYSQL_STMT after reconnect. Kea checks that this pointer is not
NULL before using the statement.

diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc
index f4a05637f3bd326b3ba9c603f7b1dbaa61ac547b..756a620deb653207387d663d8cc89f361095a2c5 100644 (file)
--- a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc
+++ b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc
@@ -82,18 +82,6 @@ MySqlConfigBackendImpl(const DatabaseConnection::ParameterMap& parameters,
     }
 }
 
-MySqlConfigBackendImpl::~MySqlConfigBackendImpl() {
-    // Free up the prepared statements, ignoring errors. (What would we do
-    // about them? We're destroying this object and are not really concerned
-    // with errors on a database connection that is about to go away.)
-    for (int i = 0; i < conn_.statements_.size(); ++i) {
-        if (conn_.statements_[i] != NULL) {
-            (void) mysql_stmt_close(conn_.statements_[i]);
-            conn_.statements_[i] = NULL;
-        }
-    }
-}
-
 MySqlBindingPtr
 MySqlConfigBackendImpl::createBinding(const Triplet<uint32_t>& triplet) {
     if (triplet.unspecified()) {

diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.h b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.h
index 2422a601c7b36efa954f94b9e7832332a4983788..edf31c6c0469b09aa8c17b1963a6449df4d7236c 100644 (file)
--- a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.h
+++ b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.h
@@ -113,7 +113,7 @@ public:
                                     const db::DbCallback db_reconnect_callback);
 
     /// @brief Destructor.
-    virtual ~MySqlConfigBackendImpl();
+    virtual ~MySqlConfigBackendImpl() {};
 
     /// @brief Creates MySQL binding from an @c Optional of integer type.
     ///

diff --git a/src/lib/dhcpsrv/mysql_host_data_source.cc b/src/lib/dhcpsrv/mysql_host_data_source.cc
index af1ffc9400a580af9fb1efb7421c2bf8af5d48a4..b39ae5402991c9277fe4d2b2349d3ebe9e8959c1 100644 (file)
--- a/src/lib/dhcpsrv/mysql_host_data_source.cc
+++ b/src/lib/dhcpsrv/mysql_host_data_source.cc
@@ -2937,11 +2937,11 @@ MySqlHostDataSourceImpl::addStatement(MySqlHostContextPtr& ctx,
                                       StatementIndex stindex,
                                       std::vector<MYSQL_BIND>& bind) {
     // Bind the parameters to the statement
-    int status = mysql_stmt_bind_param(ctx->conn_.statements_[stindex], &bind[0]);
+    int status = mysql_stmt_bind_param(ctx->conn_.getStatement(stindex), &bind[0]);
     checkError(ctx, status, stindex, "unable to bind parameters");
 
     // Execute the statement
-    status = MysqlExecuteStatement(ctx->conn_.statements_[stindex]);
+    status = MysqlExecuteStatement(ctx->conn_.getStatement(stindex));
 
     if (status != 0) {
         // Failure: check for the special case of duplicate entry.
@@ -2956,7 +2956,7 @@ MySqlHostDataSourceImpl::addStatement(MySqlHostContextPtr& ctx,
     // index in the database. Unique indexes are not created in the database
     // when it may be sometimes allowed to insert duplicated records per
     // server's configuration.
-    my_ulonglong numrows = mysql_stmt_affected_rows(ctx->conn_.statements_[stindex]);
+    my_ulonglong numrows = mysql_stmt_affected_rows(ctx->conn_.getStatement(stindex));
     if (numrows == 0) {
         isc_throw(DuplicateEntry, "Database duplicate entry error");
     }
@@ -2967,18 +2967,18 @@ MySqlHostDataSourceImpl::delStatement(MySqlHostContextPtr& ctx,
                                       StatementIndex stindex,
                                       MYSQL_BIND* bind) {
     // Bind the parameters to the statement
-    int status = mysql_stmt_bind_param(ctx->conn_.statements_[stindex], &bind[0]);
+    int status = mysql_stmt_bind_param(ctx->conn_.getStatement(stindex), &bind[0]);
     checkError(ctx, status, stindex, "unable to bind parameters");
 
     // Execute the statement
-    status = MysqlExecuteStatement(ctx->conn_.statements_[stindex]);
+    status = MysqlExecuteStatement(ctx->conn_.getStatement(stindex));
 
     if (status != 0) {
         checkError(ctx, status, stindex, "unable to execute");
     }
 
     // Let's check how many hosts were deleted.
-    my_ulonglong numrows = mysql_stmt_affected_rows(ctx->conn_.statements_[stindex]);
+    my_ulonglong numrows = mysql_stmt_affected_rows(ctx->conn_.getStatement(stindex));
 
     return (numrows != 0);
 }
@@ -3046,30 +3046,30 @@ MySqlHostDataSourceImpl::getHostCollection(MySqlHostContextPtr& ctx,
                                            bool single) const {
 
     // Bind the selection parameters to the statement
-    int status = mysql_stmt_bind_param(ctx->conn_.statements_[stindex], bind);
+    int status = mysql_stmt_bind_param(ctx->conn_.getStatement(stindex), bind);
     checkError(ctx, status, stindex, "unable to bind WHERE clause parameter");
 
     // Set up the MYSQL_BIND array for the data being returned and bind it to
     // the statement.
     std::vector<MYSQL_BIND> outbind = exchange->createBindForReceive();
-    status = mysql_stmt_bind_result(ctx->conn_.statements_[stindex], &outbind[0]);
+    status = mysql_stmt_bind_result(ctx->conn_.getStatement(stindex), &outbind[0]);
     checkError(ctx, status, stindex, "unable to bind SELECT clause parameters");
 
     // Execute the statement
-    status = MysqlExecuteStatement(ctx->conn_.statements_[stindex]);
+    status = MysqlExecuteStatement(ctx->conn_.getStatement(stindex));
     checkError(ctx, status, stindex, "unable to execute");
 
     // Ensure that all the lease information is retrieved in one go to avoid
     // overhead of going back and forth between client and server.
-    status = mysql_stmt_store_result(ctx->conn_.statements_[stindex]);
+    status = mysql_stmt_store_result(ctx->conn_.getStatement(stindex));
     checkError(ctx, status, stindex, "unable to set up for storing all results");
 
     // Set up the fetch "release" object to release resources associated
     // with the call to mysql_stmt_fetch when this method exits, then
     // retrieve the data. mysql_stmt_fetch return value equal to 0 represents
     // successful data fetch.
-    MySqlFreeResult fetch_release(ctx->conn_.statements_[stindex]);
-    while ((status = mysql_stmt_fetch(ctx->conn_.statements_[stindex])) ==
+    MySqlFreeResult fetch_release(ctx->conn_.getStatement(stindex));
+    while ((status = mysql_stmt_fetch(ctx->conn_.getStatement(stindex))) ==
            MLM_MYSQL_FETCH_SUCCESS) {
         try {
             exchange->processFetchedData(result);

diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.cc b/src/lib/dhcpsrv/mysql_lease_mgr.cc
index dfdb097f5b6e6dde0178117262bb22dc5ff86a93..a49fff455555d5289114be30b0abcde868f71d06 100644 (file)
--- a/src/lib/dhcpsrv/mysql_lease_mgr.cc
+++ b/src/lib/dhcpsrv/mysql_lease_mgr.cc
@@ -1713,7 +1713,7 @@ private:
                       " - invalid statement index" << statement_index_);
         }
 
-        statement_ = conn_.statements_[statement_index_];
+        statement_ = conn_.getStatement(statement_index_);
     }
 
     /// @brief Database connection to use to execute the query
@@ -1949,11 +1949,11 @@ MySqlLeaseMgr::addLeaseCommon(MySqlLeaseContextPtr& ctx,
                               std::vector<MYSQL_BIND>& bind) {
 
     // Bind the parameters to the statement
-    int status = mysql_stmt_bind_param(ctx->conn_.statements_[stindex], &bind[0]);
+    int status = mysql_stmt_bind_param(ctx->conn_.getStatement(stindex), &bind[0]);
     checkError(ctx, status, stindex, "unable to bind parameters");
 
     // Execute the statement
-    status = MysqlExecuteStatement(ctx->conn_.statements_[stindex]);
+    status = MysqlExecuteStatement(ctx->conn_.getStatement(stindex));
     if (status != 0) {
 
         // Failure: check for the special case of duplicate entry.  If this is
@@ -2051,31 +2051,31 @@ MySqlLeaseMgr::getLeaseCollection(MySqlLeaseContextPtr& ctx,
 
     if (bind) {
         // Bind the selection parameters to the statement
-        status = mysql_stmt_bind_param(ctx->conn_.statements_[stindex], bind);
+        status = mysql_stmt_bind_param(ctx->conn_.getStatement(stindex), bind);
         checkError(ctx, status, stindex, "unable to bind WHERE clause parameter");
     }
 
     // Set up the MYSQL_BIND array for the data being returned and bind it to
     // the statement.
     std::vector<MYSQL_BIND> outbind = exchange->createBindForReceive();
-    status = mysql_stmt_bind_result(ctx->conn_.statements_[stindex], &outbind[0]);
+    status = mysql_stmt_bind_result(ctx->conn_.getStatement(stindex), &outbind[0]);
     checkError(ctx, status, stindex, "unable to bind SELECT clause parameters");
 
     // Execute the statement
-    status = MysqlExecuteStatement(ctx->conn_.statements_[stindex]);
+    status = MysqlExecuteStatement(ctx->conn_.getStatement(stindex));
     checkError(ctx, status, stindex, "unable to execute");
 
     // Ensure that all the lease information is retrieved in one go to avoid
     // overhead of going back and forth between client and server.
-    status = mysql_stmt_store_result(ctx->conn_.statements_[stindex]);
+    status = mysql_stmt_store_result(ctx->conn_.getStatement(stindex));
     checkError(ctx, status, stindex, "unable to set up for storing all results");
 
     // Set up the fetch "release" object to release resources associated
     // with the call to mysql_stmt_fetch when this method exits, then
     // retrieve the data.
-    MySqlFreeResult fetch_release(ctx->conn_.statements_[stindex]);
+    MySqlFreeResult fetch_release(ctx->conn_.getStatement(stindex));
     int count = 0;
-    while ((status = mysql_stmt_fetch(ctx->conn_.statements_[stindex])) == 0) {
+    while ((status = mysql_stmt_fetch(ctx->conn_.getStatement(stindex))) == 0) {
         try {
             result.push_back(exchange->getLeaseData());
 
@@ -2809,16 +2809,16 @@ MySqlLeaseMgr::updateLeaseCommon(MySqlLeaseContextPtr& ctx,
                                  const LeasePtr& lease) {
 
     // Bind the parameters to the statement
-    int status = mysql_stmt_bind_param(ctx->conn_.statements_[stindex], bind);
+    int status = mysql_stmt_bind_param(ctx->conn_.getStatement(stindex), bind);
     checkError(ctx, status, stindex, "unable to bind parameters");
 
     // Execute
-    status = MysqlExecuteStatement(ctx->conn_.statements_[stindex]);
+    status = MysqlExecuteStatement(ctx->conn_.getStatement(stindex));
     checkError(ctx, status, stindex, "unable to execute");
 
     // See how many rows were affected.  The statement should only update a
     // single row.
-    int affected_rows = mysql_stmt_affected_rows(ctx->conn_.statements_[stindex]);
+    int affected_rows = mysql_stmt_affected_rows(ctx->conn_.getStatement(stindex));
 
     // Check success case first as it is the most likely outcome.
     if (affected_rows == 1) {
@@ -2951,16 +2951,16 @@ MySqlLeaseMgr::deleteLeaseCommon(StatementIndex stindex,
     MySqlLeaseContextPtr ctx = get_context.ctx_;
 
     // Bind the input parameters to the statement
-    int status = mysql_stmt_bind_param(ctx->conn_.statements_[stindex], bind);
+    int status = mysql_stmt_bind_param(ctx->conn_.getStatement(stindex), bind);
     checkError(ctx, status, stindex, "unable to bind WHERE clause parameter");
 
     // Execute
-    status = MysqlExecuteStatement(ctx->conn_.statements_[stindex]);
+    status = MysqlExecuteStatement(ctx->conn_.getStatement(stindex));
     checkError(ctx, status, stindex, "unable to execute");
 
     // See how many rows were affected.  Note that the statement may delete
     // multiple rows.
-    return (static_cast<uint64_t>(mysql_stmt_affected_rows(ctx->conn_.statements_[stindex])));
+    return (static_cast<uint64_t>(mysql_stmt_affected_rows(ctx->conn_.getStatement(stindex))));
 }
 
 bool

diff --git a/src/lib/mysql/mysql_connection.h b/src/lib/mysql/mysql_connection.h
index f68a42c5dec4e57ed4ae2051d52c8e00516d86d3..e1342464bf5ab45ec84842101c962c05a90308ce 100644 (file)
--- a/src/lib/mysql/mysql_connection.h
+++ b/src/lib/mysql/mysql_connection.h
@@ -304,6 +304,24 @@ public:
     /// @brief Clears prepared statements and text statements.
     void clearStatements();
 
+    /// @brief Returns a prepared statement by an index
+    ///
+    /// @tparam StatementIndex Type of the statement index enum.
+    ///
+    /// @param index Statement index.
+    /// @return Pointer to the prepared statement.
+    /// @throw isc::db::DbConnectionUnusable when the @c mysql pointer in the
+    ///        returned statement is NULL; it may be the result of the database
+    ///        connectivity loss.
+    template<typename StatementIndex>
+    MYSQL_STMT* getStatement(StatementIndex index) const {
+        if (statements_[index]->mysql == 0) {
+            isc_throw(db::DbConnectionUnusable,
+                      "MySQL pointer for the prepared statement is NULL as a result of connectivity loss");
+        }
+        return (statements_[index]);
+    }
+
     /// @brief Open Database
     ///
     /// Opens the database using the information supplied in the parameters
@@ -436,7 +454,7 @@ public:
         int status = 0;
         if (!in_bind_vec.empty()) {
             // Bind parameters to the prepared statement.
-            status = mysql_stmt_bind_param(statements_[index],
+            status = mysql_stmt_bind_param(getStatement(index),
                                            in_bind_vec.empty() ? 0 : &in_bind_vec[0]);
             checkError(status, index, "unable to bind parameters for select");
         }
@@ -447,20 +465,20 @@ public:
             out_bind_vec.push_back(out_binding->getMySqlBinding());
         }
         if (!out_bind_vec.empty()) {
-            status = mysql_stmt_bind_result(statements_[index], &out_bind_vec[0]);
+            status = mysql_stmt_bind_result(getStatement(index), &out_bind_vec[0]);
             checkError(status, index, "unable to bind result parameters for select");
         }
 
         // Execute query.
-        status = MysqlExecuteStatement(statements_[index]);
+        status = MysqlExecuteStatement(getStatement(index));
         checkError(status, index, "unable to execute");
 
-        status = mysql_stmt_store_result(statements_[index]);
+        status = mysql_stmt_store_result(getStatement(index));
         checkError(status, index, "unable to set up for storing all results");
 
         // Fetch results.
-        MySqlFreeResult fetch_release(statements_[index]);
-        while ((status = mysql_stmt_fetch(statements_[index])) ==
+        MySqlFreeResult fetch_release(getStatement(index));
+        while ((status = mysql_stmt_fetch(getStatement(index))) ==
                MLM_MYSQL_FETCH_SUCCESS) {
             try {
                 // For each returned row call user function which should
@@ -511,12 +529,12 @@ public:
         }
 
         // Bind the parameters to the statement
-        int status = mysql_stmt_bind_param(statements_[index],
+        int status = mysql_stmt_bind_param(getStatement(index),
                                            in_bind_vec.empty() ? 0 : &in_bind_vec[0]);
         checkError(status, index, "unable to bind parameters");
 
         // Execute the statement
-        status = MysqlExecuteStatement(statements_[index]);
+        status = MysqlExecuteStatement(getStatement(index));
 
         if (status != 0) {
             // Failure: check for the special case of duplicate entry.
@@ -555,12 +573,12 @@ public:
         }
 
         // Bind the parameters to the statement
-        int status = mysql_stmt_bind_param(statements_[index],
+        int status = mysql_stmt_bind_param(getStatement(index),
                                            in_bind_vec.empty() ? 0 : &in_bind_vec[0]);
         checkError(status, index, "unable to bind parameters");
 
         // Execute the statement
-        status = MysqlExecuteStatement(statements_[index]);
+        status = MysqlExecuteStatement(getStatement(index));
 
         if (status != 0) {
             // Failure: check for the special case of duplicate entry.
@@ -581,7 +599,7 @@ public:
         }
 
         // Let's return how many rows were affected.
-        return (static_cast<uint64_t>(mysql_stmt_affected_rows(statements_[index])));
+        return (static_cast<uint64_t>(mysql_stmt_affected_rows(getStatement(index))));
     }
 
     /// @brief Commits current transaction
@@ -731,14 +749,14 @@ private:
     template<typename T>
     void setIntParameterValue(const std::string& name, int64_t min, int64_t max, T& value);
 
-public:
-
     /// @brief Prepared statements
     ///
-    /// This field is public, because it is used heavily from MySqlConnection
-    /// and will be from MySqlHostDataSource.
+    /// The statements should be accessed using the @c getStatement method because
+    /// it checks whether the returned statement is valid.
     std::vector<MYSQL_STMT*> statements_;
 
+public:
+
     /// @brief Raw text of statements
     ///
     /// This field is public, because it is used heavily from MySqlConnection

diff --git a/src/lib/mysql/tests/mysql_connection_unittest.cc b/src/lib/mysql/tests/mysql_connection_unittest.cc
index cc2b21d3ea43b6f4290090e986dfca41ba2982dc..b2ac654552d632d1ca99224d1fc3f0ae1354e574 100644 (file)
--- a/src/lib/mysql/tests/mysql_connection_unittest.cc
+++ b/src/lib/mysql/tests/mysql_connection_unittest.cc
@@ -6,6 +6,7 @@
 
 #include <config.h>
 
+#include <database/database_connection.h>
 #include <exceptions/exceptions.h>
 #include <mysql/mysql_connection.h>
 #include <mysql/testutils/mysql_schema.h>
@@ -741,6 +742,16 @@ TEST_F(MySqlConnectionTest, writeTimeoutZero) {
 
 #endif  // HAVE_MYSQL_GET_OPTION
 
+
+// Tests that the statement can be accessed by index.
+TEST_F(MySqlConnectionTest, getStatement) {
+    auto statement0 = conn_.getStatement(0);
+    ASSERT_TRUE(statement0);
+    auto statement1 = conn_.getStatement(1);
+    ASSERT_TRUE(statement1);
+    EXPECT_NE(statement0, statement1);
+}
+
 TEST_F(MySqlConnectionWithPrimaryKeyTest, select) {
     select();
 }