Skip to content

Handle PHP-bug with single char vars#205

Merged
Nyholm merged 1 commit into
php-cache:masterfrom
Jaumo:single-char-var
Dec 20, 2017
Merged

Handle PHP-bug with single char vars#205
Nyholm merged 1 commit into
php-cache:masterfrom
Jaumo:single-char-var

Conversation

@brstgt
Copy link
Copy Markdown
Contributor

@brstgt brstgt commented Dec 19, 2017

We encounter an issue with single-char PHP vars in production (running on amphp/aerys). We observe regularly, that single-char-vars become undefined. After PHP restart everything is ok again. The bug definitely exists in 7.1.9 + 7.1.11
Fixing that bug is hard, a workaround is simple and does not harm.

Thanks!

Question Answer
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets comma-separated list of tickets fixed by the PR, if any
License MIT
Doc PR reference to the documentation PR, if any

Description

TODO

  • Add tests
  • Add documentation
  • Updated Changelog.md

We encounter an issue with single-char PHP vars in production (running on amphp/aerys). We observe regularly, that single-char-vars become undefined. After PHP restart everything is ok again. The bug definitely exists in 7.1.9 + 7.1.11
Fixing that bug is hard, a workaround is simple and does not harm.

Thanks!
@prisis
Copy link
Copy Markdown
Contributor

prisis commented Dec 19, 2017

Hey :)
is this the only place?

@codecov-io
Copy link
Copy Markdown

codecov-io commented Dec 19, 2017

Codecov Report

Merging #205 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #205   +/-   ##
=========================================
  Coverage     81.95%   81.95%           
  Complexity      598      598           
=========================================
  Files            31       31           
  Lines          1524     1524           
=========================================
  Hits           1249     1249           
  Misses          275      275
Impacted Files Coverage Δ Complexity Δ
src/Adapter/Common/CacheItem.php 97.67% <100%> (ø) 36 <0> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 615fcd2...07bf6e7. Read the comment docs.

@brstgt
Copy link
Copy Markdown
Contributor Author

brstgt commented Dec 19, 2017

Most probably not, but it's one that affects us. Curiosly, our own code contains zillions of things like "Exception $e". We never observed logs from these but what happens on a regular basis is like

$f = new Foo();
$f->bar();

Results in
[E_NOTICE] Undefined variable: f
+
Call to a member function bar() on null

Yes, I am serious :(

@prisis
Copy link
Copy Markdown
Contributor

prisis commented Dec 19, 2017

Can you add the 2 php version to travis?
So that we can see if it really fails or you have some custom setting in your php version.

And can you please change than all of the places where something like this exists

@prisis
Copy link
Copy Markdown
Contributor

prisis commented Dec 19, 2017

If you take a look on symfony code, they have many single-char names for functions

And im not against it, to change the single-chars to a normal name

Copy link
Copy Markdown
Member

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh. Interesting bug.

@brstgt
Copy link
Copy Markdown
Contributor Author

brstgt commented Dec 20, 2017

@prisis Adding more php versions to travis won't prove anything unfortunately. We cannot reproduce that in a deterministic way. We observe that bug only in long running programs after an indefinite amount of time.
We never observed that in PHPFPM as this creates a PHP runtime for every single request. I am 99.9% sure it is a bug in the PHP memory management.
WHEN we observe that, ALL single-char vars (at least in certain places) cause that problem from the moment of the first occurence until the process is restarted.

Btw.: I opened a PHP issue for that https://bugs.php.net/bug.php?id=75710&thanks=4

@Nyholm
Copy link
Copy Markdown
Member

Nyholm commented Dec 20, 2017

On the top of my head, this is the only place we use a single char var like this.

I’m also not sure of adding these PHP versions to Travis. Travis does not even support all patch versions.

@brstgt
Copy link
Copy Markdown
Contributor Author

brstgt commented Dec 20, 2017

@Nyholm Then I'd be happy about a merge :) Actually it's not a big deal, right?

@Nyholm Nyholm merged commit 52aa833 into php-cache:master Dec 20, 2017
@Nyholm
Copy link
Copy Markdown
Member

Nyholm commented Dec 20, 2017

Thank you.
I'll do a search to see if we have anymore of these one char vars

prisis pushed a commit to PrisisForks/cache that referenced this pull request Mar 26, 2018
We encounter an issue with single-char PHP vars in production (running on amphp/aerys). We observe regularly, that single-char-vars become undefined. After PHP restart everything is ok again. The bug definitely exists in 7.1.9 + 7.1.11
Fixing that bug is hard, a workaround is simple and does not harm.

Thanks!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants