Page MenuHomePhabricator

Instances of User are not serializable!
Closed, ResolvedPublicPRODUCTION ERROR

Description

Error

MediaWiki version: 1.36.0-wmf.11

message
Instances of User are not serializable!

Impact

Serializing user objects can drag along large amounts of data. Deserialized user objects may contain stale data. This is the case especially for the following member fields of User:

  • mRequest A WebRequest that would be invalid and misleading when deserialized during a later request.
  • mBlock, mBlockedFromCreateAccount, $mGlobalBlock:AbstractBlock instances that may be inaccurate at a later time.

Notes

One referer: https://fr.wikisource.org/w/index.php?title=Page:Revue_des_Deux_Mondes_-_1833_-_tome_4.djvu/362&action=edit

It seems that there are at least two things that cause User to be serialized:

  • ProofreadPage's PageContent gets serialized by PageEditStash. It contains a PageLevel, which contains a User object. This could be replaced with a UserIdentityValue, at least for serialization. PageContent actually already has special code for handing the User object when serializing to JSON for storage. This just isn't used when it's serialized via PHP's native serialization for caching. We could consider using proper content serialization in PageEditStash. Filed as T264389.
  • FeaturedFeedChannel gets serialized by FeatureFeeds for caching. It contains a User object, which is used to construct ParserOptions in the getFeedItem() method. The solution here would be to pass a User option (or ParserOptiosn) to getFeedItem() as a parameter. Filed as T264391.

Details

Request ID
90dc0671-65c3-4705-943f-5d420ec7e791
Request URL
https://fr.wikisource.org/w/api.php
Stack Trace
exception.trace
#0 [internal function]: User->__sleep()
#1 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MemcachedPeclBagOStuff.php(423): serialize(stdClass)
#2 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MediumSpecificBagOStuff.php(987): MemcachedPeclBagOStuff->serialize(stdClass)
#3 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MediumSpecificBagOStuff.php(788): MediumSpecificBagOStuff->getSerialized(stdClass, string)
#4 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MediumSpecificBagOStuff.php(172): MediumSpecificBagOStuff->makeValueOrSegmentList(string, stdClass, integer, integer)
#5 /srv/mediawiki/php-1.36.0-wmf.11/includes/Storage/PageEditStash.php(479): MediumSpecificBagOStuff->set(string, stdClass, integer, integer)
#6 /srv/mediawiki/php-1.36.0-wmf.11/includes/Storage/PageEditStash.php(160): MediaWiki\Storage\PageEditStash->storeStashValue(string, ProofreadPage\Page\PageContent, ParserOutput, string, User)
#7 /srv/mediawiki/php-1.36.0-wmf.11/includes/api/ApiStashEdit.php(151): MediaWiki\Storage\PageEditStash->parseAndCache(WikiPage, ProofreadPage\Page\PageContent, User, string)
#8 /srv/mediawiki/php-1.36.0-wmf.11/includes/api/ApiMain.php(1565): ApiStashEdit->execute()
#9 /srv/mediawiki/php-1.36.0-wmf.11/includes/api/ApiMain.php(545): ApiMain->executeAction()
#10 /srv/mediawiki/php-1.36.0-wmf.11/includes/api/ApiMain.php(516): ApiMain->executeActionWithErrorHandling()
#11 /srv/mediawiki/php-1.36.0-wmf.11/api.php(90): ApiMain->execute()
#12 /srv/mediawiki/php-1.36.0-wmf.11/api.php(45): wfApiMain()
#13 /srv/mediawiki/w/api.php(3): require(string)
#14 {main}

Event Timeline

There doesn't appear to be a user object being serialized though?

// Store what is actually needed and split the output into another key (T204742)
$stashInfo = (object)[
	'pstContent' => $pstContent,
	'output'     => $parserOutput,
	'timestamp'  => $timestamp,
	'edits'      => $user->getEditCount()
];
$ok = $this->cache->set( $key, $stashInfo, $ttl, BagOStuff::WRITE_ALLOW_SEGMENTS );

Change 631536 had a related patch set uploaded (by Urbanecm; owner: Urbanecm):
[mediawiki/core@master] Remove NonSerializableTrait from User object

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

There doesn't appear to be a user object being serialized though?

// Store what is actually needed and split the output into another key (T204742)
$stashInfo = (object)[
	'pstContent' => $pstContent,
	'output'     => $parserOutput,
	'timestamp'  => $timestamp,
	'edits'      => $user->getEditCount()
];
$ok = $this->cache->set( $key, $stashInfo, $ttl, BagOStuff::WRITE_ALLOW_SEGMENTS );

maybe $pstContent or $parserOutput have a user object in there by accident?

https://gerrit.wikimedia.org/g/mediawiki/core/+/e7ff3cbb6b4db6f4db55746422d64d4205720f79/includes/Storage/PageEditStash.php#160

PageEditStash.php
			$code = $this->storeStashValue(
				$key,
				$editInfo->pstContent,
				$editInfo->output,
				$editInfo->timestamp,
				$user
			);

https://gerrit.wikimedia.org/g/mediawiki/core/+/e7ff3cbb6b4db6f4db55746422d64d4205720f79/includes/Storage/PageEditStash.php#160

PageEditStash.php
			$code = $this->storeStashValue(
				$key,
				$editInfo->pstContent,
				$editInfo->output,
				$editInfo->timestamp,
				$user
			);

Yes but within storeStashValue the user is only used to retrieve the edit count

yeah I don't know what's going on. it must be the parser output that has a user in it somehow?

Change 631477 had a related patch set uploaded (by 20after4; owner: Urbanecm):
[mediawiki/core@wmf/1.36.0-wmf.11] Remove NonSerializableTrait from User object

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

This isn't used only in storeStashvalue through, a different instance is $availableFeeds = array_keys( FeaturedFeeds::getFeeds( false, $this->getUser() ) ); which can easily make user to get serialized. I confirm it gets serialized in FeaturedFeeds::getFeeds at least. Not sure how it happened in stash edit logic through :/.

Call chain:

  • PageEditStash->parseAndCache takes User $user
  • PageEditStash->prepareContentForEdit
  • WikiPage->prepareContentForEdit -> getDerivedDataUpdater() -> getPreparedEdit()
  • DerivedPageDataUpdater->getPreparedEdit()
    • $preparedEdit = new PreparedEdit();
    • $preparedEdit->popts = $this->getCanonicalParserOptions();

It looks like ParserOptions might be storing a User object.

Mentioned in SAL (#wikimedia-operations) [2020-10-01T21:29:53Z] <twentyafterfour@deploy1001> rebuilt and synchronized wikiversions files: rollback group0 as well T264363

yeah I don't know what's going on. it must be the parser output that has a user in it somehow?

Not natively, but some extension might be stuffing something into setExtensionData() or setProperty(). Maybe we should start logging the types of things that go in there.

Change 631477 merged by jenkins-bot:
[mediawiki/core@wmf/1.36.0-wmf.11] Remove NonSerializableTrait from User object

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

Change 631536 merged by jenkins-bot:
[mediawiki/core@master] Remove NonSerializableTrait from User object

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

It looks like ParserOptions might be storing a User object.

Likely, but I don't see how ParserOptions would end up in the object that is going into the cache.

It looks like ParserOptions might be storing a User object.

Likely, but I don't see how ParserOptions would end up in the object that is going into the cache.

ParserOptions is stored as part of the PreparedEdit object, right?

EDIT: Ah, I see. We only store part of PreparedEdit, not the whole object. Hmn.. I'm not sure in that case.

#6 /srv/mediawiki/php-1.36.0-wmf.11/includes/Storage/PageEditStash.php(160): MediaWiki\Storage\PageEditStash->storeStashValue(string, ProofreadPage\Page\PageContent, ParserOutput, string, User)

ProofreadPage\Page\PageContent
/**
	 * @param WikitextContent $header
	 * @param WikitextContent $body
	 * @param WikitextContent $footer
	 * @param PageLevel $level <--------------
	 */
	public function __construct(
		WikitextContent $header, WikitextContent $body, WikitextContent $footer, PageLevel $level
	) {
ProofreadPage\Page\PageLevel
/**
	 * @var User|null last user of the page
	 */
	protected $user = null;
	/**
	 * @param int $level
	 * @param User|null $user
	 */
	public function __construct( $level = self::NOT_PROOFREAD, User $user = null ) {
		$this->level = $level;
		$this->user = $user; // <--------------------------
	}

Isn't it?

FTR, we also have different traces on logstash:

Variant 1
#0 [internal function]: User->__sleep()
#1 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MemcachedPeclBagOStuff.php(254): Memcached->add(string, array, integer)
#2 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MediumSpecificBagOStuff.php(235): MemcachedPeclBagOStuff->doAdd(string, array, integer, integer)
#3 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MediumSpecificBagOStuff.php(311): MediumSpecificBagOStuff->add(string, array, integer, integer)
#4 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MediumSpecificBagOStuff.php(266): MediumSpecificBagOStuff->mergeViaCas(string, Closure, integer, integer, integer)
#5 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/wancache/WANObjectCache.php(762): MediumSpecificBagOStuff->merge(string, Closure, integer, integer)
#6 /srv/mediawiki/php-1.36.0-wmf.11/extensions/FeaturedFeeds/includes/FeaturedFeeds.php(58): WANObjectCache->set(string, array, integer)
#7 /srv/mediawiki/php-1.36.0-wmf.11/extensions/FeaturedFeeds/includes/FeaturedFeeds.php(119): FeaturedFeeds::getFeeds(string, User)
#8 /srv/mediawiki/php-1.36.0-wmf.11/includes/HookContainer/HookContainer.php(333): FeaturedFeeds::beforePageDisplay(OutputPage, SkinMinerva)
#9 /srv/mediawiki/php-1.36.0-wmf.11/includes/HookContainer/HookContainer.php(140): MediaWiki\HookContainer\HookContainer->callLegacyHook(string, array, array, array)
#10 /srv/mediawiki/php-1.36.0-wmf.11/includes/HookContainer/HookRunner.php(987): MediaWiki\HookContainer\HookContainer->run(string, array, array)
#11 /srv/mediawiki/php-1.36.0-wmf.11/includes/OutputPage.php(2610): MediaWiki\HookContainer\HookRunner->onBeforePageDisplay(OutputPage, SkinMinerva) ///// <-- any skin class can end up here, seen also for Vector, MB,Timeless, and Modern
#12 /srv/mediawiki/php-1.36.0-wmf.11/includes/MediaWiki.php(947): OutputPage->output(boolean)
#13 /srv/mediawiki/php-1.36.0-wmf.11/includes/MediaWiki.php(960): MediaWiki->{closure}()
#14 /srv/mediawiki/php-1.36.0-wmf.11/includes/MediaWiki.php(543): MediaWiki->main()
#15 /srv/mediawiki/php-1.36.0-wmf.11/index.php(53): MediaWiki->run()
#16 /srv/mediawiki/php-1.36.0-wmf.11/index.php(46): wfIndexMain()
#17 /srv/mediawiki/w/index.php(3): require(string)
#18 {main}
Variant 2
#0 [internal function]: User->__sleep()
#1 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MemcachedPeclBagOStuff.php(254): Memcached->add(string, array, integer)
#2 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MediumSpecificBagOStuff.php(235): MemcachedPeclBagOStuff->doAdd(string, array, integer, integer)
#3 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MediumSpecificBagOStuff.php(311): MediumSpecificBagOStuff->add(string, array, integer, integer)
#4 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MediumSpecificBagOStuff.php(266): MediumSpecificBagOStuff->mergeViaCas(string, Closure, integer, integer, integer)
#5 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/wancache/WANObjectCache.php(762): MediumSpecificBagOStuff->merge(string, Closure, integer, integer)
#6 /srv/mediawiki/php-1.36.0-wmf.11/extensions/FeaturedFeeds/includes/FeaturedFeeds.php(58): WANObjectCache->set(string, array, integer)
#7 /srv/mediawiki/php-1.36.0-wmf.11/extensions/FeaturedFeeds/includes/SpecialFeedItem.php(19): FeaturedFeeds::getFeeds(string, User)
#8 /srv/mediawiki/php-1.36.0-wmf.11/includes/specialpage/SpecialPage.php(600): SpecialFeedItem->execute(string)
#9 /srv/mediawiki/php-1.36.0-wmf.11/includes/specialpage/SpecialPageFactory.php(709): SpecialPage->run(string)
#10 /srv/mediawiki/php-1.36.0-wmf.11/includes/MediaWiki.php(307): MediaWiki\SpecialPage\SpecialPageFactory->executePath(Title, RequestContext)
#11 /srv/mediawiki/php-1.36.0-wmf.11/includes/MediaWiki.php(940): MediaWiki->performRequest()
#12 /srv/mediawiki/php-1.36.0-wmf.11/includes/MediaWiki.php(543): MediaWiki->main()
#13 /srv/mediawiki/php-1.36.0-wmf.11/index.php(53): MediaWiki->run()
#14 /srv/mediawiki/php-1.36.0-wmf.11/index.php(46): wfIndexMain()
#15 /srv/mediawiki/w/index.php(3): require(string)
#16 {main}
Variant 3
#0 [internal function]: User->__sleep()
#1 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MemcachedPeclBagOStuff.php(254): Memcached->add(string, array, integer)
#2 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MediumSpecificBagOStuff.php(235): MemcachedPeclBagOStuff->doAdd(string, array, integer, integer)
#3 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MediumSpecificBagOStuff.php(311): MediumSpecificBagOStuff->add(string, array, integer, integer)
#4 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MediumSpecificBagOStuff.php(266): MediumSpecificBagOStuff->mergeViaCas(string, Closure, integer, integer, integer)
#5 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/wancache/WANObjectCache.php(762): MediumSpecificBagOStuff->merge(string, Closure, integer, integer)
#6 /srv/mediawiki/php-1.36.0-wmf.11/extensions/FeaturedFeeds/includes/FeaturedFeeds.php(58): WANObjectCache->set(string, array, integer)
#7 /srv/mediawiki/php-1.36.0-wmf.11/extensions/FeaturedFeeds/includes/ApiFeaturedFeeds.php(49): FeaturedFeeds::getFeeds(string, User)
#8 /srv/mediawiki/php-1.36.0-wmf.11/includes/api/ApiBase.php(1705): ApiFeaturedFeeds->getAllowedParams(integer)
#9 /srv/mediawiki/php-1.36.0-wmf.11/includes/api/ApiBase.php(730): ApiBase->getFinalParams()
#10 /srv/mediawiki/php-1.36.0-wmf.11/includes/api/ApiMain.php(1156): ApiBase->extractRequestParams()
#11 /srv/mediawiki/php-1.36.0-wmf.11/includes/api/ApiMain.php(1544): ApiMain->setupModule()
#12 /srv/mediawiki/php-1.36.0-wmf.11/includes/api/ApiMain.php(545): ApiMain->executeAction()
#13 /srv/mediawiki/php-1.36.0-wmf.11/includes/api/ApiMain.php(516): ApiMain->executeActionWithErrorHandling()
#14 /srv/mediawiki/php-1.36.0-wmf.11/api.php(90): ApiMain->execute()
#15 /srv/mediawiki/php-1.36.0-wmf.11/api.php(45): wfApiMain()
#16 /srv/mediawiki/w/api.php(3): require(string)
#17 {main}
Variant 4
#0 [internal function]: User->__sleep()
#1 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MemcachedPeclBagOStuff.php(254): Memcached->add(string, array, integer)
#2 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MediumSpecificBagOStuff.php(235): MemcachedPeclBagOStuff->doAdd(string, array, integer, integer)
#3 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MediumSpecificBagOStuff.php(311): MediumSpecificBagOStuff->add(string, array, integer, integer)
#4 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MediumSpecificBagOStuff.php(266): MediumSpecificBagOStuff->mergeViaCas(string, Closure, integer, integer, integer)
#5 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/wancache/WANObjectCache.php(762): MediumSpecificBagOStuff->merge(string, Closure, integer, integer)
#6 /srv/mediawiki/php-1.36.0-wmf.11/extensions/FeaturedFeeds/includes/FeaturedFeeds.php(58): WANObjectCache->set(string, array, integer)
#7 /srv/mediawiki/php-1.36.0-wmf.11/extensions/FeaturedFeeds/includes/ApiFeaturedFeeds.php(30): FeaturedFeeds::getFeeds(string, User)
#8 /srv/mediawiki/php-1.36.0-wmf.11/includes/api/ApiMain.php(1565): ApiFeaturedFeeds->execute()
#9 /srv/mediawiki/php-1.36.0-wmf.11/includes/api/ApiMain.php(545): ApiMain->executeAction()
#10 /srv/mediawiki/php-1.36.0-wmf.11/includes/api/ApiMain.php(516): ApiMain->executeActionWithErrorHandling()
#11 /srv/mediawiki/php-1.36.0-wmf.11/api.php(90): ApiMain->execute()
#12 /srv/mediawiki/php-1.36.0-wmf.11/api.php(45): wfApiMain()
#13 /srv/mediawiki/w/api.php(3): require(string)
#14 {main}

All of these seem to share the same root cause (FeaturedFeeds::getFeeds), and if we include the one in the task description there's no more of them.

Thanks for the analysis, @Daimona! I'll update the description.

daniel claimed this task.

Closing this, since the production error has been fixed. Follow-up work filed under T264393: Mark user object as non-serializable.

Similar issue happens again with FeaturedFeeds: T273242