Page MenuHomePhabricator

Storing objects in session data causes unnecessary session writes, and emits spurious warnings with $wgPHPSessionHandling = 'warn'
Closed, ResolvedPublic

Description

The debug warnings we've recorded in T400668 did not have anything interesting in them, so I decided to very carefully read the PHPSessionHandler::write() method, which emits these warnings, and look for holes in the logic.

I noticed that it does some === comparisons on arbitrary deserialized data, which wouldn't work as expected if that data could contain any PHP objects, and it turns out that it can. As a result, storing objects in session data causes unnecessary session writes (which probably explains at least some of the shutdown writes we've seen in T400249) and emits spurious warnings with $wgPHPSessionHandling = 'warn' (which hopefully explains all of the warnings we've seen in T400668).

Related Objects

Event Timeline

Change #1181002 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/core@master] PHPSessionHandler: Better handle objects stored in the session

https://gerrit.wikimedia.org/r/1181002

Change #1181780 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/core@master] Add test for I9d67e5b

https://gerrit.wikimedia.org/r/1181780

Change #1181002 merged by jenkins-bot:

[mediawiki/core@master] PHPSessionHandler: Better handle objects stored in the session

https://gerrit.wikimedia.org/r/1181002

Change #1181782 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/core@wmf/1.45.0-wmf.15] PHPSessionHandler: Better handle objects stored in the session

https://gerrit.wikimedia.org/r/1181782

Change #1181780 merged by jenkins-bot:

[mediawiki/core@master] Add test for I9d67e5b

https://gerrit.wikimedia.org/r/1181780

Change #1181782 merged by jenkins-bot:

[mediawiki/core@wmf/1.45.0-wmf.15] PHPSessionHandler: Better handle objects stored in the session

https://gerrit.wikimedia.org/r/1181782

Mentioned in SAL (#wikimedia-operations) [2025-08-26T13:20:42Z] <lucaswerkmeister-wmde@deploy1003> Started scap sync-world: Backport for [[gerrit:1181782|PHPSessionHandler: Better handle objects stored in the session (T402602)]], [[gerrit:1181788|Add maint script to fix global edit count of renamed users (T313900)]], [[gerrit:1181789|Add maint script to fix wrong actors in local log entries for global renames (T398177)]]

Mentioned in SAL (#wikimedia-operations) [2025-08-26T13:26:59Z] <lucaswerkmeister-wmde@deploy1003> matmarex, lucaswerkmeister-wmde: Backport for [[gerrit:1181782|PHPSessionHandler: Better handle objects stored in the session (T402602)]], [[gerrit:1181788|Add maint script to fix global edit count of renamed users (T313900)]], [[gerrit:1181789|Add maint script to fix wrong actors in local log entries for global renames (T398177)]] synced to the testservers (see https://wikitech.wikim

Mentioned in SAL (#wikimedia-operations) [2025-08-26T13:33:37Z] <lucaswerkmeister-wmde@deploy1003> Finished scap sync-world: Backport for [[gerrit:1181782|PHPSessionHandler: Better handle objects stored in the session (T402602)]], [[gerrit:1181788|Add maint script to fix global edit count of renamed users (T313900)]], [[gerrit:1181789|Add maint script to fix wrong actors in local log entries for global renames (T398177)]] (duration: 12m 54s)