Question

I have a CMS that is failing when more than one person is logged in. I think the problem may be that I'm not closing databases properly. Here is the basic setup:

I have a master class called Mysql. The __construct() method looks something like this:

class Mysql {

    protected $conn;

    function __construct() {

        if(!$this->conn) $this->conn = New mysqli(SITE_HOSTNAME, SITE_USERNAME, SITE_PASSWORD, SITE_DATABASE);

}

All of my classes are descendants of this class, and so whenever they are instantiated, they get their own connection.

I'm using prepared statements to protect against sql injection. A sample method might look like this:

function foo($blog = 0) {

    $query = "SELECT catid, catname
              FROM category
              WHERE blog = ?
              ORDER BY catname ASC";

    $result = array();
    if($stmt = $this->conn->prepare($query)) {
        $stmt->bind_param('i', $blog);
        $stmt->execute();
        $stmt->bind_result($catid, $catname);
        while($stmt->fetch()) $result[$catid] = stripslashes($catname);
        $stmt->close();
        return $result;
    }

}

The Mysql __destruct() method looks like this:

function __destruct() {
    if($this->conn) $this->conn->close();
}

Is my problem that I am not closing my connections properly, or that I do not have enough active connections?

My provider says that I have 30 simultaneous connections available. Not enough?

Thanks for your help.

Was it helpful?

Solution

The Problem

All of my classes are descendants of this class, and so whenever they are instantiated, they get their own connection.

The problem with the way you are doing it is that every single instance of any class that inherits Mysql is opening a connection to the Mysql server that lives until the request is completed, or the variable destructed (unless you are killing these objects manually, that's at the end of the request). If you have 30 instance of classes like this, then you have maxed out your connections.

Fixing The Problem

Your classes shouldn't all be inheriting this Mysql class that represents your interface to the database.

Instead, you should be passing a single instance of this Mysql class where it's needed.

There are two ways this could be done effectively:

Dependency Injection

class SomeClass {
    protected $db;

    public function __construct(Mysql &$db) {
        $this->db = $db;
    }

    public function functionThatNeedsDatabase() {
        $this->db->doSomethingWithTheDatabase();
    }
}

$db = new Mysql(); //A single instance of Mysql class has been created.
$someClass = new SomeClass($db);
$someClass->functionThatNeedsDatabase();

Pass to Method When Needed

class SomeClass {
    public function functionThatNeedsDatabase(Mysql &$db) {
        $db->doSomethingWithTheDatabase();
    }
}

$db = new Mysql(); //A single instance
$someClass = new SomeClass();
$someClass->functionThatNeedsDatabase($db);

OTHER TIPS

Your problem is "All of my classes are descendants of this class, and so whenever they are instantiated, they get their own connection." In PHP connection are automatically closed when the script ends. You really only need 1 connection per user. You should create an instance of your DB class and pass that object into all the classes that need to use it (Dependency Injection pattern).

30 connections is not a whole lot, but that is sufficient for a lot of traffic. I would avoid using persistent connections. Read the documentation thoroughly on that, there are plenty of warnings.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top