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

Docblock typedef crash #10739

Closed
weirdan opened this issue Feb 23, 2024 · 13 comments · Fixed by #10856
Closed

Docblock typedef crash #10739

weirdan opened this issue Feb 23, 2024 · 13 comments · Fixed by #10856

Comments

@weirdan
Copy link
Collaborator

weirdan commented Feb 23, 2024

          For me it is still crashing but it seem to fix one problem.

As pointing out above I had the problem already since at least 5.21.1. With your branch I see multiple catches on that introduced point and then a fallback to the reported monolog\logger class from 5.21.1 instead of mockery.

Here is the stack trace in case this shows anything new.

Uncaught Exception: InvalidArgumentException Could not get class storage for monolog\logger
Emitted in /var/www/project/vendor/vimeo/psalm/src/Psalm/Internal/Provider/ClassLikeStorageProvider.php:45
Stack trace in the forked worker:
#0 /var/www/project/vendor/vimeo/psalm/src/Psalm/Internal/Type/TypeExpander.php(292): Psalm\Internal\Provider\ClassLikeStorageProvider->get()
#1 /var/www/project/vendor/vimeo/psalm/src/Psalm/Internal/Type/TypeParser.php(1697): Psalm\Internal\Type\TypeExpander::expandAtomic()
#2 /var/www/project/vendor/vimeo/psalm/src/Psalm/Internal/Type/TypeParser.php(661): Psalm\Internal\Type\TypeParser::resolveTypeAliases()
#3 /var/www/project/vendor/vimeo/psalm/src/Psalm/Internal/Type/TypeParser.php(188): Psalm\Internal\Type\TypeParser::getTypeFromGenericTree()
#4 /var/www/project/vendor/vimeo/psalm/src/Psalm/Internal/Type/TypeParser.php(158): Psalm\Internal\Type\TypeParser::getTypeFromTree()
#5 /var/www/project/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/CommentAnalyzer.php(163): Psalm\Internal\Type\TypeParser::parseTokens()
#6 /var/www/project/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/CommentAnalyzer.php(63): Psalm\Internal\Analyzer\CommentAnalyzer::arrayToDocblocks()
#7 /var/www/project/vendor/vimeo/psalm/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php(1580): Psalm\Internal\Analyzer\CommentAnalyzer::getTypeFromComment()
#8 /var/www/project/vendor/vimeo/psalm/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php(778): Psalm\Internal\PhpVisitor\Reflector\ClassLikeNodeScanner->visitPropertyDeclaration()
#9 /var/www/project/vendor/vimeo/psalm/src/Psalm/Internal/PhpVisitor/ReflectorVisitor.php(155): Psalm\Internal\PhpVisitor\Reflector\ClassLikeNodeScanner->start()
#10 /var/www/project/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(200): Psalm\Internal\PhpVisitor\ReflectorVisitor->enterNode()
#11 /var/www/project/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(114): PhpParser\NodeTraverser->traverseArray()
#12 /var/www/project/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(223): PhpParser\NodeTraverser->traverseNode()
#13 /var/www/project/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(91): PhpParser\NodeTraverser->traverseArray()
#14 /var/www/project/vendor/vimeo/psalm/src/Psalm/Internal/Scanner/FileScanner.php(79): PhpParser\NodeTraverser->traverse()
#15 /var/www/project/vendor/vimeo/psalm/src/Psalm/Internal/Codebase/Scanner.php(554): Psalm\Internal\Scanner\FileScanner->scan()
#16 /var/www/project/vendor/vimeo/psalm/src/Psalm/Internal/Codebase/Scanner.php(782): Psalm\Internal\Codebase\Scanner->scanFile()
#17 /var/www/project/vendor/vimeo/psalm/src/Psalm/Internal/Fork/Pool.php(191): Psalm\Internal\Codebase\Scanner->scanAPath()
#18 /var/www/project/vendor/vimeo/psalm/src/Psalm/Internal/Codebase/Scanner.php(332): Psalm\Internal\Fork\Pool->__construct()
#19 /var/www/project/vendor/vimeo/psalm/src/Psalm/Internal/Codebase/Scanner.php(280): Psalm\Internal\Codebase\Scanner->scanFilePaths()
#20 /var/www/project/vendor/vimeo/psalm/src/Psalm/Codebase.php(505): Psalm\Internal\Codebase\Scanner->scanFiles()
#21 /var/www/project/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/ProjectAnalyzer.php(510): Psalm\Codebase->scanFiles()
#22 /var/www/project/vendor/vimeo/psalm/src/Psalm/Internal/Cli/Psalm.php(374): Psalm\Internal\Analyzer\ProjectAnalyzer->check()
#23 /var/www/project/vendor/vimeo/psalm/psalm(9): Psalm\Internal\Cli\Psalm::run()
#24 {main} in /var/www/project/vendor/vimeo/psalm/src/Psalm/Internal/Fork/Pool.php:379

Originally posted by @simonberger in #10706 (comment)

@weirdan weirdan changed the title Docblock (?) intersection crash Docblock typedef crash Feb 23, 2024
@weirdan
Copy link
Collaborator Author

weirdan commented Feb 23, 2024

@boesing would you mind providing more info here? From the backtrace it looks like it crashed while reading a docblock for a property on a class that had type aliases, and that property docblock mentioned Monolog\Logger (this one is from the backtrace you posted on slack).

@boesing
Copy link
Contributor

boesing commented Feb 23, 2024

@weirdan I was trying to get more details with --debug-by-line but since there are that many files being scanned, its almost impossible to find the originating class.
I was searching our codebase for the usage of explicit Logger but we do only depend on PSR-3 LoggerInterface (but using monolog under the hood).

If you have some more hidden "features" which could help me identifying the problem, I'd happy to help.

@weirdan
Copy link
Collaborator Author

weirdan commented Feb 23, 2024

What's the last few lines starting with Parsing|Scanning before the crash?

@simonberger
Copy link

I also got not helpful hint from the --debug output. All the previous files seem not be related. I can try again and with --debug-by-line next week.

@simonberger
Copy link

I did not try to build a reproducer because that might be very tricky with multithreading involved, but I debugged it in my project and tracked it down to a specific type in monolog.

In use is monolog/monolog 2.9.2

It always crashes on comments handling an imported type. What I can do for psalm to successfully scan is either to remove all type imports related to Level:

There are more classes importing this file, but I guess unrelated in my project, which is using TestHandler directly

Or I can remove these 3 comments related to the type:

With this information I stepped some more Minutes through some code, but I lack too much internal knowledge to understand the problem and find a possible fix in a reasonable time.

I hope this can help.

@weirdan
Copy link
Collaborator Author

weirdan commented Feb 23, 2024

That's consistent with the backtrace, thanks. Can you also check if replacing references to constants with their values here: https://github.com/Seldaek/monolog/blob/437cb3628f4cf6042cc10ae97fc2b8472e48ca1f/src/Monolog/Logger.php#L30 helps with the crash? Constants and typedefs are double fun to debug.

@simonberger
Copy link

No, it does not help.

@AtCliffUnderline
Copy link

AtCliffUnderline commented Mar 15, 2024

Hi there!
Have same issue, can not really give the reproducer because I dont understand what to cut from my production repo.
However, tried to find cause with git bisect and it leads me to commit 5731f92

Could it be the problem? Reproduces only with multithreading (--threads=8 in my case)

@boesing
Copy link
Contributor

boesing commented Mar 15, 2024

I can confirm that the commit mentioned is the one which introduced that issue. I outlined that on symfony slack some weeks ago. Same issue for me that there is almost no way of extracting a reproducer from our repository as that is kinda huge.
Maybe I find some time just deleting random code from our repository to see if it is related to vendor/ packages or actually written code in our repository.

@simonberger
Copy link

Thanks for the hint about the causing commit @AtCliffUnderline
I tested a little bit and the problematic addition are these lines

if ($type_aliases !== []) {
    $intersection_types = self::resolveTypeAliases(
        $codebase,
        $generic_params[0]->getAtomicTypes(),
    );

    if ($intersection_types !== []) {
        $generic_params[0] = $generic_params[0]->setTypes($intersection_types);
    }
}

Commenting it out showed no crash anymore, but more interestingly there were zero changes done by $generic_params[0]->setTypes because the type matched always.
Commenting just this line out still lead to a crash.
I was confused at that point because self::resolveTypeAliases shouldn't change anything and I couldn't even spot any cache or similar.
Possibly I missed something or this code is not thread safe.

@AtCliffUnderline
Copy link

@weirdan This closed as completed, but I just reproduced this on 5.23.1.

Maybe reopen?

@simonberger
Copy link

@AtCliffUnderline there is not yet a release with this fix.

@AtCliffUnderline
Copy link

Oh wow missed that, sorry!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants