Improve Database::__destruct() and add a __clone() method too
authorAaron Schulz <aschulz@wikimedia.org>
Tue, 20 Sep 2016 16:17:14 +0000 (09:17 -0700)
committerAaron Schulz <aschulz@wikimedia.org>
Tue, 20 Sep 2016 22:04:11 +0000 (22:04 +0000)
* Close dangling connections in LoadBalancer/Database destructors.
* When DBs are cloned, create new connection resources for the clone
  so the two do Database objects don't clobber each other.

Change-Id: I3adb57cbb1fdc2a17e6d95389d0562ef22701576

includes/libs/rdbms/database/Database.php
includes/libs/rdbms/loadbalancer/LoadBalancer.php

index 9b4e4ac..33e36b5 100644 (file)
@@ -3487,6 +3487,26 @@ abstract class Database implements IDatabase, LoggerAwareInterface {
                return (string)$this->mConn;
        }
 
+       /**
+        * Make sure that copies do not share the same client binding handle
+        * @throws DBConnectionError
+        */
+       public function __clone() {
+               $this->connLogger->debug(
+                       "Cloning " . get_class( $this ) . " is not recomended; forking connection:\n" .
+                       ( new RuntimeException() )->getTraceAsString()
+               );
+
+               if ( $this->isOpen() ) {
+                       // Open a new connection resource without messing with the old one
+                       $this->mOpened = false;
+                       $this->mConn = false;
+                       $this->mTrxLevel = 0; // no trx anymore
+                       $this->open( $this->mServer, $this->mUser, $this->mPassword, $this->mDBname );
+                       $this->lastPing = microtime( true );
+               }
+       }
+
        /**
         * Called by serialize. Throw an exception when DB connection is serialized.
         * This causes problems on some database engines because the connection is
@@ -3498,7 +3518,7 @@ abstract class Database implements IDatabase, LoggerAwareInterface {
        }
 
        /**
-        * Run a few simple sanity checks
+        * Run a few simple sanity checks and close dangling connections
         */
        public function __destruct() {
                if ( $this->mTrxLevel && $this->mTrxDoneWrites ) {
@@ -3510,5 +3530,12 @@ abstract class Database implements IDatabase, LoggerAwareInterface {
                        $fnames = implode( ', ', $danglingWriters );
                        trigger_error( "DB transaction writes or callbacks still pending ($fnames)." );
                }
+
+               if ( $this->mConn ) {
+                       // Avoid connection leaks for sanity
+                       $this->closeConnection();
+                       $this->mConn = false;
+                       $this->mOpened = false;
+               }
        }
 }
index 791e5ad..c78f0ae 100644 (file)
@@ -1460,4 +1460,9 @@ class LoadBalancer implements ILoadBalancer {
                        $db->tablePrefix( $prefix );
                } );
        }
+
+       function __destruct() {
+               // Avoid connection leaks for sanity
+               $this->closeAll();
+       }
 }