Yii2: Logging breaks transaction when something with db connection is logged

[pawelkania]

Logging breaks transaction when something with db connection is logged. BaseVarDumper::exportInternal is using serialize which is closing db connection (calling close inside __sleep)

What steps will reproduce the problem?

Start transaction, do some changes in db and then log something including db connection.

$transaction = \Yii::$app->db->beginTransaction();
// do some changes in db
(new LocationZip([
            'zip' => 12345,
            'federation' => 'Sachsen'
]))->save();

// see changes exists in db
$newRecord = LocationZip::find()->where(['zip' => 12345,])->one();


// log something that has db connection inside (it could be stacktrace of exception)
\Yii::info(\Yii::$app->db);

// connection to db was closed while trying to serialize log message so transaction is lost
$newRecord = LocationZip::find()->where(['zip' => 12345,])->one();

// will throw exception because transaction doesn't exists anymore
$transaction->commit();

What is the expected result?

Logging should't do side effects like closing db connections

Additional info

| Q | A
| ---------------- | ---
| Yii version | 2.0.12
| PHP version | 7.1
| Operating system | Ubuntu

[bscheshirwork]

May be resolve it with

\Yii::info(clone \Yii::$app->db);

?

[pawelkania]

@bscheshirwork It was only simple example, your solution won't work if db connection is somewhere in logged stacktrace.

[bscheshirwork]

i.e. title can read as "Serialize breaks transaction anything"

[cebe]

related to #13890

[rob006]

Why __sleep() closes connection?

[cebe]

to not serialize the PDO instance. This is mainly to know we are not connected after unserializing the connection. See https://github.com/yiisoft/yii2/pull/10149

[rob006]

This does not seem right to me. Serializing object should not affects it's state. unserialize(serialize($db)) should be equivalent to clone $db;

[klimov-paul]

This does not seem right to me. Serializing object should not affects it's state. unserialize(serialize($db)) should be equivalent to clone $db;

This is impossible, since any PHP resource including DB connection can not be shared between different PHP processes.

[klimov-paul]

This is mainly to know we are not connected after unserializing the connection.

No actual connection closing is perfromed at Connection::close() - it simply unsets the related PDO object - nullifies it. In case there is another reference to this PDO object - the connection will still stand.
Only nullifing of the last reference to the PDO object will cause its actual destruction an thus DB connection close.
Thus in general the issue described here should not ever appear.

[rob006]

This is impossible, since any PHP resource including DB connection can not be shared between different PHP processes.

Then cloned $db should open its own connection to DB. If I have two separate Connection instances, they should have separate connections to DB. I don't see any reason to magically close connections from source instance.

[klimov-paul]

@rob006, see https://stackoverflow.com/questions/15620012/serialize-file-resource for instance.

[rob006]

@klimov-paul Sorry, don't get your point. If I do $db1 = clone $db; I will have two Connection instances with with two separate connections to DB. Connection of $db will not be magically closed, $db1 will just open new one. It should work the same in case of unserialize(serialize($db)).

[klimov-paul]

Statement unserialize(serialize($db)) has absolutely no purpose. It is unlikely you need to unserialize the serialized object withing the same process. Object serialization is performed to save object for the another process processing, thus serialization process should be aware of such possibility and prevent passing stream resources between different processes.

[rob006]

This is just a example. I'm not talking about serializing resources, but about not closing resource that is used by existing object. Serializing object should not affect its instance.

[klimov-paul]

Unit test to reproduce

abstract class ConnectionTest extends DatabaseTestCase
{
    // ...

    public function testSerialize()
    {
        $connection = $this->getConnection(false, false);
        $connection->open();
        $serialized = serialize($connection);

        $this->assertNotNull($connection->pdo);

        $unserialized = unserialize($serialized);
        $this->assertInstanceOf('yii\db\Connection', $unserialized);
        $this->assertNull($unserialized->pdo);

        $this->assertEquals(123, $unserialized->createCommand('SELECT 123')->queryScalar());
    }
}

For me this is an unexpected PHP behavior as __sleep() applied to the instance itself, but it is as it is.
The only possible solution I can see is unsetting PDO at __wakeup() instead of __sleep().

[klimov-paul]

No go - ends up with Exception:

PDOException: You cannot serialize or unserialize PDO instances

[rob006]

There is a example in documentation how to handle serialization and PDO: http://php.net/manual/en/language.oop5.magic.php#object.sleep

$pdo attribute should not be serialized.

[klimov-paul]

Yes, this should do the trick.

[klimov-paul]

Resolved by commit 9e6f9e3b6d41c918718b953968cf631c6a03ad73