Browse Source

[2821] Small memory leak issue in MySQLLeaseMgr constructor

the mysql lease manager wouldn't close the mysql database immediately if initialization failed at certain points.

This proposed fix is not entirely complete; it would probably be better to either use such a holder class as the member, or to slightly clean it up so that the return value from release() can be given to mysql_ (and therefore use the holder in the construction code instead of mysql_), but I did not want to touch too much existing code at this point.
Jelte Jansen 12 years ago
parent
commit
21dae0aa02
1 changed files with 40 additions and 10 deletions
  1. 40 10
      src/lib/dhcpsrv/mysql_lease_mgr.cc

+ 40 - 10
src/lib/dhcpsrv/mysql_lease_mgr.cc

@@ -193,6 +193,32 @@ TaggedStatement tagged_statements[] = {
     {MySqlLeaseMgr::NUM_STATEMENTS, NULL}
 };
 
+// Small RAII object for safer initialization, will close the database
+// connection upon destruction, unless release() has been called.
+class MySQLHolder {
+public:
+    MySQLHolder(MYSQL* mysql) : mysql_(mysql) {}
+
+    ~MySQLHolder() {
+        if (mysql_) {
+            mysql_close(mysql_);
+        }
+    }
+
+    MYSQL* get_ptr() {
+        return (mysql_);
+    }
+
+    MYSQL* release() {
+        MYSQL* ptr = mysql_;
+        mysql_ = NULL;
+        return (ptr);
+    }
+
+private:
+    MYSQL* mysql_;
+};
+
 };  // Anonymous namespace
 
 
@@ -289,7 +315,7 @@ public:
         memset(hwaddr_buffer_, 0, sizeof(hwaddr_buffer_));
         memset(client_id_buffer_, 0, sizeof(client_id_buffer_));
         std::fill(&error_[0], &error_[LEASE_COLUMNS], MLM_FALSE);
- 
+
         // Set the column names (for error messages)
         columns_[0] = "address";
         columns_[1] = "hwaddr";
@@ -536,7 +562,7 @@ public:
         memset(addr6_buffer_, 0, sizeof(addr6_buffer_));
         memset(duid_buffer_, 0, sizeof(duid_buffer_));
         std::fill(&error_[0], &error_[LEASE_COLUMNS], MLM_FALSE);
- 
+
         // Set the column names (for error messages)
         columns_[0] = "address";
         columns_[1] = "duid";
@@ -809,7 +835,7 @@ private:
     // schema.
     // Note: arrays are declared fixed length for speed of creation
     std::string     addr6_;             ///< String form of address
-    char            addr6_buffer_[ADDRESS6_TEXT_MAX_LEN + 1];  ///< Character 
+    char            addr6_buffer_[ADDRESS6_TEXT_MAX_LEN + 1];  ///< Character
                                         ///< array form of V6 address
     unsigned long   addr6_length_;      ///< Length of the address
     MYSQL_BIND      bind_[LEASE_COLUMNS]; ///< Bind array
@@ -870,14 +896,15 @@ private:
     MYSQL_STMT*     statement_;     ///< Statement for which results are freed
 };
 
-
 // MySqlLeaseMgr Constructor and Destructor
 
-MySqlLeaseMgr::MySqlLeaseMgr(const LeaseMgr::ParameterMap& parameters) 
+MySqlLeaseMgr::MySqlLeaseMgr(const LeaseMgr::ParameterMap& parameters)
     : LeaseMgr(parameters), mysql_(NULL) {
 
-    // Allocate context for MySQL - it is destroyed in the destructor.
-    mysql_ = mysql_init(NULL);
+    // Allocate context for MySQL
+    MySQLHolder mysql_holder(mysql_init(NULL));
+
+    mysql_ = mysql_holder.get_ptr();
     if (mysql_ == NULL) {
         isc_throw(DbOpenError, "unable to initialize MySQL");
     }
@@ -902,6 +929,9 @@ MySqlLeaseMgr::MySqlLeaseMgr(const LeaseMgr::ParameterMap& parameters)
     // program and the database.
     exchange4_.reset(new MySqlLease4Exchange());
     exchange6_.reset(new MySqlLease6Exchange());
+
+    // Let the real destructor take care of cleaning up now
+    mysql_holder.release();
 }
 
 
@@ -970,7 +1000,7 @@ MySqlLeaseMgr::convertFromDatabaseTime(const MYSQL_TIME& expire,
     expire_tm.tm_hour = expire.hour;
     expire_tm.tm_min = expire.minute;
     expire_tm.tm_sec = expire.second;
-    expire_tm.tm_isdst = -1;    // Let the system work out about DST 
+    expire_tm.tm_isdst = -1;    // Let the system work out about DST
 
     // Convert to local time
     cltt = mktime(&expire_tm) - valid_lifetime;
@@ -1022,7 +1052,7 @@ MySqlLeaseMgr::openDatabase() {
     }
 
     // Set options for the connection:
-    // 
+    //
     // Automatic reconnection: after a period of inactivity, the client will
     // disconnect from the database.  This option causes it to automatically
     // reconnect when another operation is about to be done.
@@ -1088,7 +1118,7 @@ MySqlLeaseMgr::prepareStatements() {
     // Allocate space for all statements
     statements_.clear();
     statements_.resize(NUM_STATEMENTS, NULL);
-    
+
     text_statements_.clear();
     text_statements_.resize(NUM_STATEMENTS, std::string(""));