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

Redis::connection causes segmentation fault #1883

Open
Phil-Barker opened this issue Mar 15, 2024 · 7 comments
Open

Redis::connection causes segmentation fault #1883

Phil-Barker opened this issue Mar 15, 2024 · 7 comments
Labels
bug Something isn't working

Comments

@Phil-Barker
Copy link

Phil-Barker commented Mar 15, 2024

  • Larastan Version: 2.9.2
  • --level used: 6
  • Pull request with failing test: N/A private code

Description

Interacting with Redis::connection causes segmentation fault

This seems to be related to phpstan/phpstan#6300 which was fixed in
phpstan/phpstan-src@9ab4e6f

Running tests with just phpstan pass fine, but with the larastan extension included it causes a segmentation fault anywhere that Redis::connection is used.

I'm using phpstan 1.10.62

Laravel code where the issue was found

        $redis = Redis::connection('session');
        $redis->del($basket->redisName . ':' . Session()->get('accountUser')->id);
@mad-briller
Copy link
Contributor

can you run with xdebug enabled and the --debug flag as mentioned in the issue you linked and provide the stack trace? it's likely another infinite recursion bug in a larastan extension and that would make it easier to track down

@Phil-Barker
Copy link
Author

Yes of course.

Running with xdebug it does indeed say infinite recursion has been detected.
The stack trace is huge, so attaching as a file

stacktrace.txt

I've taken out the base path as it relates to a project under NDA, hope this helps though

@Phil-Barker
Copy link
Author

Minimal code to reproduce

<?php

namespace App\Listeners\Basket;

use App\Events\AccountUserChanged;
use Illuminate\Support\Facades\Redis;
use Illuminate\Support\Facades\Session;

class GetBasket
{
    public function handle(AccountUserChanged $event): void
    {
        $accountUser = Session::get('accountUser');
        $redis = Redis::connection('session');
        $basket = $redis->get('basket:' . $accountUser->id);
    }
}

@calebdw
Copy link
Contributor

calebdw commented Mar 15, 2024

So you don't have to manually parse through the stack trace, here's what the gpt has to say:

The recurring pattern involves these components:

PHPStan\Type\ObjectType->hasMethod() calls leading to
PHPStan\Reflection\Mixin\MixinMethodsClassReflectionExtension->findMethod() and back to
PHPStan\Type\ObjectType->hasMethod()
This cycle suggests that there's an issue within the handling of method existence checks and finding methods in mixed types or extensions, likely causing the infinite loop.

More specifically, the loop seems to start around entries #474 to #489, where you have:

PHPStan\Reflection\ClassReflection->hasMethod() calling
PHPStan\Type\ObjectType->hasMethod(), which then calls
PHPStan\Reflection\Mixin\MixinMethodsClassReflectionExtension->findMethod(), and the process repeats.
The recursion might be due to the way MixinMethodsClassReflectionExtension interacts with ObjectType when trying to resolve whether a class has a specific method. This could be triggered by a complex type hierarchy or misuse of mixins that PHPStan tries to analyze, leading to a loop.

@szepeviktor szepeviktor added the bug Something isn't working label Mar 15, 2024
@promatik
Copy link

promatik commented May 2, 2024

Not only Redis::connection, but Redis::sIsMember or Redis::sMembers or any other static method cause this error.

I had to move all Redis methods to a trait file so I could exclude just that file from stan.

@Phil-Barker this should of course be fixed upstream, but in the meanwhile you can fix your stan like this;

includes:
    - ./vendor/larastan/larastan/extension.neon

parameters:

    paths:
        - ...
    excludePaths:
        - app/Models/Traits/RedisTrait.php

RedisTrait.php

use Illuminate\Support\Facades\Redis;

trait RedisTrait
{
    private function redisIsMember(string $key, string|int $value): bool
    {
        return Redis::sIsMember($key, (string) $value);
    }

    private function redisMembers(string $key): array
    {
        return Redis::sMembers($key);
    }
}

@calebdw
Copy link
Contributor

calebdw commented May 15, 2024

@Phil-Barker,

PHPStan v1.11.1 includes a couple of fixes for recursion, could you check if this is still an issue?

sdkawata added a commit to sdkawata/larastan that referenced this issue May 15, 2024
sdkawata added a commit to sdkawata/larastan that referenced this issue May 15, 2024
sdkawata added a commit to sdkawata/larastan that referenced this issue May 15, 2024
@promatik
Copy link

@calebdw

PHPStan v1.11.1 includes a couple of fixes for recursion, could you check if this is still an issue?

I tried with the latest php 8.3, and phpstan 1.11.1 and it still times out...

Note: Using configuration file phpstan.neon.
 1073/1113 [==========================>-]  96%The following exception is caused by a process timeout
Check https://getcomposer.org/doc/06-config.md#process-timeout for details

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants