[#2792] 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.

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()) {

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.
     ///

src/lib/dhcpsrv/mysql_host_data_source.cc

@@ -3042,11 +3042,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.
@@ -3061,7 +3061,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");
     }
@@ -3072,18 +3072,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);
 }
@@ -3151,30 +3151,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]);
+    MySqlFreeResult fetch_release(ctx->conn_.getStatement(stindex));
-    while ((status = mysql_stmt_fetch(ctx->conn_.statements_[stindex])) ==
+    while ((status = mysql_stmt_fetch(ctx->conn_.getStatement(stindex))) ==
            MLM_MYSQL_FETCH_SUCCESS) {
         try {
             exchange->processFetchedData(result);

src/lib/dhcpsrv/mysql_lease_mgr.cc

@@ -2081,7 +2081,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
@@ -2366,11 +2366,11 @@ MySqlLeaseMgr::addLeaseCommon(MySqlLeaseContextPtr& 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.  If this is
@@ -2484,31 +2484,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());
 
@@ -3241,16 +3241,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) {
@@ -3412,16 +3412,16 @@ MySqlLeaseMgr::deleteLeaseCommon(MySqlLeaseContextPtr& ctx,
                                  StatementIndex stindex,
                                  MYSQL_BIND* bind) {
     // 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
@@ -3903,11 +3903,11 @@ MySqlLeaseMgr::deleteRelayId6(const IOAddress& addr) {
     StatementIndex stindex = DELETE_RELAY_ID6;
 
     // 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");
 }
 
@@ -3933,11 +3933,11 @@ MySqlLeaseMgr::deleteRemoteId6(const IOAddress& addr) {
     StatementIndex stindex = DELETE_REMOTE_ID6;
 
     // 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");
 }
 
@@ -3977,11 +3977,11 @@ MySqlLeaseMgr::addRelayId6(const IOAddress& lease_addr,
     StatementIndex stindex = ADD_RELAY_ID6;
 
     // 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");
 }
 
@@ -4021,11 +4021,11 @@ MySqlLeaseMgr::addRemoteId6(const IOAddress& lease_addr,
     StatementIndex stindex = ADD_REMOTE_ID6;
 
     // 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");
 }
 
@@ -4807,13 +4807,13 @@ MySqlLeaseMgr::wipeExtendedInfoTables6() {
     MySqlLeaseContextPtr ctx = get_context.ctx_;
 
     StatementIndex stindex = WIPE_RELAY_ID6;
-    int status = MysqlExecuteStatement(ctx->conn_.statements_[stindex]);
+    int status = MysqlExecuteStatement(ctx->conn_.getStatement(stindex));
     if (status != 0) {
         checkError(ctx, status, stindex, "unable to execute");
     }
 
     stindex = WIPE_REMOTE_ID6;
-    status = MysqlExecuteStatement(ctx->conn_.statements_[stindex]);
+    status = MysqlExecuteStatement(ctx->conn_.getStatement(stindex));
     if (status != 0) {
         checkError(ctx, status, stindex, "unable to execute");
     }
@@ -4835,22 +4835,22 @@ MySqlLeaseMgr::byRelayId6size() const {
     bind[0].buffer_type = MYSQL_TYPE_LONGLONG;
     bind[0].buffer = reinterpret_cast<char*>(&count);
 
-    int status = mysql_stmt_bind_result(ctx->conn_.statements_[stindex], &bind[0]);
+    int status = mysql_stmt_bind_result(ctx->conn_.getStatement(stindex), &bind[0]);
     checkError(ctx, status, stindex, "unable to bind SELECT clause parameters");
 
     // Execute.
-    status = MysqlExecuteStatement(ctx->conn_.statements_[stindex]);
+    status = MysqlExecuteStatement(ctx->conn_.getStatement(stindex));
     if (status != 0) {
         checkError(ctx, status, stindex, "unable to execute");
     }
 
-    status = mysql_stmt_store_result(ctx->conn_.statements_[stindex]);
+    status = mysql_stmt_store_result(ctx->conn_.getStatement(stindex));
     checkError(ctx, status, stindex, "unable to store result");
 
     // Fetch the result.
-    MySqlFreeResult fetch_release(ctx->conn_.statements_[stindex]);
+    MySqlFreeResult fetch_release(ctx->conn_.getStatement(stindex));
 
-    status = mysql_stmt_fetch(ctx->conn_.statements_[stindex]);
+    status = mysql_stmt_fetch(ctx->conn_.getStatement(stindex));
     if (status != 0) {
         checkError(ctx, status, stindex, "unable to fetch results");
     }
@@ -4873,22 +4873,22 @@ MySqlLeaseMgr::byRemoteId6size() const {
     bind[0].buffer_type = MYSQL_TYPE_LONGLONG;
     bind[0].buffer = reinterpret_cast<char*>(&count);
 
-    int status = mysql_stmt_bind_result(ctx->conn_.statements_[stindex], &bind[0]);
+    int status = mysql_stmt_bind_result(ctx->conn_.getStatement(stindex), &bind[0]);
     checkError(ctx, status, stindex, "unable to bind SELECT clause parameters");
 
     // Execute.
-    status = MysqlExecuteStatement(ctx->conn_.statements_[stindex]);
+    status = MysqlExecuteStatement(ctx->conn_.getStatement(stindex));
     if (status != 0) {
         checkError(ctx, status, stindex, "unable to execute");
     }
 
-    status = mysql_stmt_store_result(ctx->conn_.statements_[stindex]);
+    status = mysql_stmt_store_result(ctx->conn_.getStatement(stindex));
     checkError(ctx, status, stindex, "unable to store result");
 
     // Fetch the result.
-    MySqlFreeResult fetch_release(ctx->conn_.statements_[stindex]);
+    MySqlFreeResult fetch_release(ctx->conn_.getStatement(stindex));
 
-    status = mysql_stmt_fetch(ctx->conn_.statements_[stindex]);
+    status = mysql_stmt_fetch(ctx->conn_.getStatement(stindex));
     if (status != 0) {
         checkError(ctx, status, stindex, "unable to fetch results");
     }

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]);
+        MySqlFreeResult fetch_release(getStatement(index));
-        while ((status = mysql_stmt_fetch(statements_[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
+    /// The statements should be accessed using the @c getStatement method because
-    /// and will be from MySqlHostDataSource.
+    /// 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

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,15 @@ 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();
 }