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

Warn about too low value for stream_select's tv_usec parameter #1044

Closed
hollodotme opened this issue Sep 26, 2018 · 5 comments
Closed

Warn about too low value for stream_select's tv_usec parameter #1044

hollodotme opened this issue Sep 26, 2018 · 5 comments
Assignees
Milestone

Comments

@hollodotme
Copy link

Subject Details
Issue type Feature request
Plugin Php Inspections (EA Extended)
Language level PHP4+

Current behaviour (description/screenshot:)

When using the stream_select function it is highly recommended by the official PHP documentation, that the tv_usec parameter is set to at least 200000 when tv_sec is set to 0 to keep CPU usage at a reasonable level.

There is currently no inspection warning when using a lower tv_usec value.

screen shot 2018-09-26 at 09 57 34

Expected behaviour

There should be a probable bug warning when the following conditions are met:

  • tv_sec is set to 0
  • tv_usec is set to a value lower than 200000

The warning message should contain a hint, that the given value may cause too high CPU usage.

The inspection should also apply if a static value in form of a constant is given to the function, like this:

<?php declare(strict_types=1);

class MyStream
{
    private const TV_USEC = 20000;

    # ...

    public function hasResponse() : bool
    {
        $read = [$this->resource];
        $write = $except = [];
       
        stream_select($read, $write, $except, 0, self::TV_USEC);

        return 1 === count($read);
    }
}

Environment details

PhpStorm 2018.2.3 (but all versions)

@kalessil kalessil self-assigned this Sep 26, 2018
@kalessil kalessil added this to the U-2.0.8 milestone Sep 26, 2018
@kalessil
Copy link
Owner

Uh, that's a very good catch @hollodotme. Will it be fine for you if I'll take this into Ultimate?

@hollodotme
Copy link
Author

Fine for me @kalessil. 😬

@kalessil
Copy link
Owner

Also a question: did you hit the issue in production (perhaps a ticket on GitHub)?

@hollodotme
Copy link
Author

Yes, it was a long existing bug in hollodotme/fast-cgi-client, that popped up during this discussion and was fixed in the last release.

It was quite hidden, because the tv_usec value was defined in a class constant. (That's why I mentioned the second part of this issue.) 😄

@kalessil
Copy link
Owner

Implemented!

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

No branches or pull requests

2 participants