Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[ClassLoader] Fix storing not-found classes in APC cache#18312
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
nicolas-grekas commentedMar 25, 2016
| Q | A |
|---|---|
| Branch? | 2.3 |
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | - |
| License | MIT |
| Doc PR | - |
xabbuh commentedMar 26, 2016
Did this cause issues somewhere? The docs state that the return value will also be |
nicolas-grekas commentedMar 26, 2016
Its false on failure, but also false when false is the cached value... $success is the way to check for cache hit or miss. |
xabbuh commentedMar 26, 2016
👍 ah of course Status: Reviewed |
| * @param string $class A class name to resolve to file | ||
| * | ||
| * @return string|null | ||
| * @return string|false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This does not seem correct. The other class loaders havestring|null. As they are decorated by ApcClassLoader something is wrong. If they do not return false at all, then this whole change seems irrelevant.
Tobion commentedMar 26, 2016
Status: Needs Work |
nicolas-grekas commentedMar 27, 2016
@Tobion fixed |
| $file =apcu_fetch($this->prefix.$class,$success); | ||
| if (!$success) { | ||
| apcu_store($this->prefix.$class,$file =$this->decorated->findFile($class) ?:null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
what is the point of the null condition?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Composer's ClassLoader returns false here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
then WinCache and Xcache class loaders should do the same normalization
Tobion commentedMar 27, 2016
|
nicolas-grekas commentedMar 28, 2016
@Tobion PR updated |
Tobion commentedMar 28, 2016
Thank you@nicolas-grekas. |
…nicolas-grekas)This PR was merged into the 2.3 branch.Discussion----------[ClassLoader] Fix storing not-found classes in APC cache| Q | A| ------------- | ---| Branch? | 2.3| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | -| License | MIT| Doc PR | -Commits-------106ed06 [ClassLoader] Fix storing not-found classes in APC cache