php - Error — session_destroy() — Trying to destroy uninitialized session

Question

I'm getting an error using session_destroy() in my PHP code.

The following script is on every page and if a user is signed in, it checks if the session is valid or not, killing the session if it's not.

session_start();

// check for users already signed in and check session
if (isset($_SESSION['user_id'])) {
    $uid = $_SESSION['user_id'];

    // check user_id is a valid id
    if (!is_numeric($uid) || $uid < 0) {
        session_unset();
        session_destroy();
        session_regenerate_id(true);
    }

    // if user agent is different, kill session
    if ($_SESSION['user_agent'] != $_SERVER['HTTP_USER_AGENT']) {
        session_unset();
        session_destroy();
        session_regenerate_id(true);
    }

    // if user's last login record fails to match session_id, kill session
    $SQL = "SELECT user_session FROM users_logins ";
    $SQL .= "WHERE user_id = :user_id ";
    $SQL .= "ORDER BY time_in DESC LIMIT 1;";
    $STH = $DBH_P->prepare($SQL);
    $STH->bindParam(':user_id', $uid);
    $STH->execute();
    $row = $STH->fetch();
    if ($STH->rowCount() > 0) {
        $db_sid = $row['user_session'];
    }
    if ($db_sid !== session_id()) {
        session_unset();
        session_destroy();
        session_regenerate_id(true);
    }
}

The error I receive indicates the failure is coming from the last session_destroy() call.

Am I using session_destroy() correctly or not? I have read other questions on here but most answers advise that session_start() must be used before destroying it, but I have started the session at the top, before the check begins.

Answer 1

You do some crazy stuff there (but you need to negotiate that with your own, I don't cover it in my answer), the reason why you see the error message is quite simple:

 session_regenerate_id(true);

is commanding PHP to destroy the old session. Problem is, you already did that, one line earlier:

 session_destroy();
 session_regenerate_id(true);

So just take a view from above. There is no reason in an OCD manner to throw as many functions as you see fit (but actually don't understand/know well) onto your session processing. Instead take the one function that is intended to do the job and actually process it's return value if you want to put some safety net in there actually. That would be more helpful.