Closing prepared statements

Question

I know that closing prepared statements is a suggested thing to do.

But I have a PHP script like this:

$sql = "SELECT * FROM `mytable` WHERE ...";
$stmt = $dbh->stmt_init();

if($stmt->prepare($sql))
{
    $stmt->bind_param("s", $user);

    if($stmt->execute())
    {
        $result = $stmt->get_result();
        $stmt->close();
    } else
        header("Location: .?error=unknown");
} else
    header("Location: .?error=conn");

The statement gets closed if everything is ok, but when something fails during the execution it doesn't get closed.

Should I write

else {
    $stmt->close();
    header("Location: .?error=unknown");
}

and

else {
    $stmt->close();
    header("Location: .?error=conn");
}

or, since an error occurred, I shouldn't worry about closing the statement?

Or could I even write:

$sql = "SELECT * FROM `mytable` WHERE ...";
$stmt = $dbh->stmt_init();

if($stmt->prepare($sql))
{
    $stmt->bind_param("s", $user);

    if($stmt->execute())
    {
        $result = $stmt->get_result();
    } else
        header("Location: .?error=unknown");
} else
    header("Location: .?error=conn");

/*some other code*/

$stmt->close; //close every statement at the very end of the script

or is it better to close the prepared statements right after I have finished using them to avoid any sort of bugs?

PHP has a garbage collection and it will be freed at the end of the script. But you should close it when you do not need it anymore. — Markus Zeller, Apr 20 '20 at 13:20
You should be using exit(); after each header call, to stop the processing of the script. — Jay Blanchard, Apr 20 '20 at 13:24
@JayBlanchard I know, it was just a [Minimal Reproducible Example](https://stackoverflow.com/help/minimal-reproducible-example) — , Apr 20 '20 at 13:31
@MarkusZeller so should I go for the second option? (closing each time before a redirect) — , Apr 20 '20 at 13:32
I would definitely prefer that. But as said, PHP will clean by itself on exit. — Markus Zeller, Apr 20 '20 at 13:46

score 1 · Accepted Answer · answered Apr 20 '20 at 13:33

1

The problem with your code is not the use of close(), but the error reporting, which is not enabled. You must enable mysqli error reporting and then you can simplify your code.

Your DB operations should ideally be wrapped in a function or a method and then you do not need to worry about closing anything. It will be closed for you automatically.

$sql = "SELECT * FROM `mytable` WHERE ...";
$stmt = $dbh->prepare($sql);
$stmt->bind_param("s", $user);
$stmt->execute();
$result = $stmt->get_result();
$stmt->close();

If you were to wrap it in a function, then there is no need for close().

function getResults(\mysqli $dbh, string $sql, string $types, array $params): array {
    $stmt = $dbh->prepare($sql);
    $stmt->bind_param($types, ...$params);
    $stmt->execute();
    return $stmt->get_result()->fetch_all(MYSQLI_ASSOC);
}

Then if you have an exception you should create a generic error handler, which will redirect the user to 500 page by generating HTTP 500 response code when an error happens, and will log all the exception details into a file on the server.

answered Apr 20 '20 at 13:33

Dharman

30,962
25
85
135

Thank you... so does the function implement garbage collection (including `$stmt->close()` when it returns a value? – Apr 20 '20 at 13:39
mysqli_stmt will call destructor once all the references to it are gone. When you restrict variables to a function scope, you ensure the objects exist only as long as you need them. PHP will then clean it all up. Even if it didn't, there's almost never any reason to close statement yourself. When script's execution ends all objects will be gone. I never use `close()` myself in any code. – Dharman Apr 20 '20 at 13:44
2

@Foxel the only exception if you would return $stmt from a function. In this case it won't be closed – Your Common Sense Apr 20 '20 at 13:48
@YourCommonSense basing on what Dharman said, right after you exit the scope of the function, PHP applies garbage collection on everything inside it... So I think it wouldn't be a problem even if I return a statement from a function – Apr 20 '20 at 14:17
@Foxel No, I said everything without any reference to it. If you leak the object outside by returning it then there's still a reference to it, so the destructor will not be called. – Dharman Apr 20 '20 at 14:18
1

@Foxel of course it is not a problem. It just won't be closed if returned - that's all – Your Common Sense Apr 20 '20 at 14:19

score 1 · Answer 2 · answered Apr 20 '20 at 13:35

Don't close it.

Despite what the other answer (written 10 years ago) says, as a rule you don't close a prepared statement. It's just not necessary. It will be closed automatically when the current scope will be closed. It means you don't care about closing the statement even during the script execution, let alone finishing the PHP script completely - in this case the entire database connection will be closed automatically, and it will free up all the associated resources as well.

On a side note, you are writing nearly five times more code to run a simple query than it needed. Here is how it should be done, tidy and concise:

$sql = "SELECT * FROM `mytable` WHERE ...";
$stmt->prepare($sql);
$stmt->bind_param("s", $user);
$stmt->execute();
$result = $stmt->get_result();

The thing is, your database code should never ever report its errors. There should be a distinct part of your code elsewhere, that would take care of all errors.

And no, doing an HTTP redirect in case of the application error is not the way to go. Your application should just return a proper HTTP code (5xx) to tell the client there was a problem.

Yes, this is the way to go. And also set exception handler. You can find a complete example in my [article](https://phpdelusions.net/articles/error_reporting) — Your Common Sense, Apr 20 '20 at 13:42

Closing prepared statements

2 Answers2

Linked