Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OOM when parsing a snippet #6896

Closed
danog opened this issue Mar 24, 2022 · 13 comments · Fixed by phpstan/phpstan-src#1137
Closed

OOM when parsing a snippet #6896

danog opened this issue Mar 24, 2022 · 13 comments · Fixed by phpstan/phpstan-src#1137
Labels

Comments

@danog
Copy link

danog commented Mar 24, 2022

Bug report

Encountering an OOM when parsing the following snippet:

Code snippet that reproduces the problem

(phpstan.org also OOMs and the share link button won't work)

<?php declare(strict_types = 1);


use IteratorIterator;
use LimitIterator;
use Traversable;
use ArrayObject;

/**
 * @template TKey as array-key
 * @template TValue
 *
 * @implements ArrayObject<TKey, TValue>
 *
 * Basic generic iterator, with additional helper functions.
 */
abstract class XIterator extends ArrayObject {
}

final class RandHelper {

	/**
	 * @template TRandKey as array-key
	 * @template TRandVal
	 * @template TRandList as array<TRandKey, TRandVal>|XIterator<TRandKey, TRandVal>|Traversable<TRandKey, TRandVal>
	 *
	 * @param TRandList $list
	 *
	 * @return (
	 * 		TRandList is array ? array<TRandKey, TRandVal> : (
	 * 		TRandList is XIterator ?	XIterator<TRandKey, TRandVal> :
	 * 		IteratorIterator<TRandKey, TRandVal>|LimitIterator<TRandKey, TRandVal>
	 * ))
	 */
	public static function getPseudoRandomWithUrl(
		array|XIterator|Traversable $list,
	): array|XIterator|IteratorIterator|LimitIterator {
		return $list;
	}
}

Expected output

No OOM

Did PHPStan help you today? Did it make you happy in any way?

Thank you for your work on phpstan <3

@mergeable
Copy link

mergeable bot commented Mar 24, 2022

This bug report is missing a link to reproduction on phpstan.org.

It will most likely be closed after manual review.

@herndlm
Copy link
Contributor

herndlm commented Mar 26, 2022

Oh, weird, looks like I cannot reproduce it in an AnalyserIntegrationTest, that's new for me

@danog
Copy link
Author

danog commented Mar 26, 2022

Btw, just analyzing it via phpstan analyze a.php in an empty folder triggers the issue for me.

@herndlm
Copy link
Contributor

herndlm commented Mar 26, 2022

Ok thx, yeah I guess I need to reconfigure something, or need another test or messed something else up.

@herndlm
Copy link
Contributor

herndlm commented Mar 26, 2022

Which PHP version are you using? I think this should not be related, but who knows.. I tested with 8 and 8.1
UPDATE: this is not related, it must be something config-wise, I'll continue looking..

@danog
Copy link
Author

danog commented Mar 26, 2022

I'm on PHP 8.1, to be more precise I ran phpstan by cloning the phpstan-src repo, running composer install, then creating a ../phpstanTest folder with just a.php in it and running ../phpstan-src/bin/phpstan analyze a.php.

@herndlm
Copy link
Contributor

herndlm commented Mar 26, 2022

Ok, I still don't have a nice automated test unfortunately, but manually calling phpstan analyze was enough to bisect as you mentioned, yeah. it started occurring with phpstan/phpstan-src@53563e9 / 1.4.7

I have to step away now, but can continue later. Hopefully Ondrej reads this in the meantime and has an idea why it works via AnalyserIntegrationTest :) got it, services.cacheStorage needs to be removed from the test config then it can be reproduced. that and based on the error message listing FileCacheStorage I assume this only occurs when the file-based cached is used, which is deactivated in the test.
at first look it looks like var_export of the cache data array is the culprit, weird, maybe there is like a recursive reference inside there. to be continued...

@herndlm
Copy link
Contributor

herndlm commented Mar 28, 2022

Puh, this is a hard one, I found the following places that make var_export run out of memory here.
I might be going down the wrong road, but maybe it still helps someone by finding the actual place to fix this @ondrejmirtes @arnaud-lb

recursive reference via TemplateTypeMap::createEmpty if called multiple times because $empty is cached statically

image

I just stupidly removed the static cache to see what happens, but then I found another one

GenericObject has itself as an ancestor

Via https://github.com/phpstan/phpstan-src/blob/1.5.0/src/Type/ObjectType.php#L1134 GenericObject gets itself as an ancestor at some point. And this cannot be var_exported either.

Something in PhpDocNode resolving related services / extension providers

I don't quite get this one, but the ClassReflection of the GenericObjectType cannot be var_exported either. I made some properties public to play around with it and am currently here ($data is the array that it would var_export and then put in the file cache):

var_export($data['9efeebddd6f78a2f832227e328a60ed6']->getTemplateTypeMap()->getTypes()['TRandList']->getTypes()[1]->getClassReflection()->fileTypeMapper->resolvedPhpDocBlockCache['534183887a003a372623e6ebe278bcaf']->phpDocNodeResolver->typeNodeResolver->extensionRegistryProvider, true)

I'm going to step back now, I hope somebody else can do something with that info. I'm not accustomed to generics, but I thought phpstan should never OOM, even if the docblock is incorrect or actually provoking recursion.

And yeah, as mentioned above already, it is fine if phpstan/phpstan-src@53563e9 is reverted (actually telling that the @return is invalid hmm).
UPDATE: apparently the changes in FileTypeMapper::getNameScopeMap are the culprit.

@herndlm
Copy link
Contributor

herndlm commented Mar 28, 2022

Had another thought - not sure if it really makes sense to adapt PHPStan's data structures to avoid circular references, which var_export can't handle. It might work, but this could come back any time. Actually, I think this is a bug in the file cache. But the question is what to use instead? Using serialize also won't work because e.g. Closures cannot be serialized.. Is another/better serialisation approach needed maybe?

UPDATE: tried the jms and symfony serializer packages, but they both had problems. and it feels like this is an overkill

UPDATE2: tried switching to serialize/unserialize, just to see what would make problems. interestingly only the snippet from here (because of a closure). I'll try to figure out where this closure actually is.. (UPDATE3: it was the ClassReflection again in GenericObjectType. maybe I'll just finish my adaptions and open a PR for further discussion of this approach instead of further derailing this issue ;))

@ondrejmirtes
Copy link
Member

Most likely fixed by: phpstan/phpstan-src#1137

@ondrejmirtes
Copy link
Member

@danog
Copy link
Author

danog commented Mar 29, 2022

All good on this front, thanks! :)
Now getting another issue on #6933

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants