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

Fatal error with SF2.8 and custom AppCache.php #276

Closed
marcobarcaro opened this issue Jan 18, 2016 · 24 comments
Closed

Fatal error with SF2.8 and custom AppCache.php #276

marcobarcaro opened this issue Jan 18, 2016 · 24 comments

Comments

@marcobarcaro
Copy link

Hi,
I ran into this fatal error with SF2.8 and FOS http cache bundle with custom AppCache:

Fatal error: Cannot redeclare class Symfony\Component\HttpFoundation\RequestMatcherInterface in ...\Test\app\cache\prod\classes.php on line 965
503 Service Unavailable

Line 965 is:
interface RequestMatcherInterface

Other files are:
// app.php
$kernel = new AppKernel('prod', false);
$kernel->loadClassCache();
$kernel = new AppCache($kernel);

// AppCache.php
use FOS\HttpCacheBundle\SymfonyCache\EventDispatchingHttpCache;
class AppCache extends EventDispatchingHttpCache
{
public function getOptions()
{
return [];
}
}

With regular SF2.8 AppCache.php everything is fine:
// AppCache.php
use Symfony\Bundle\FrameworkBundle\HttpCache\HttpCache;
class AppCache extends HttpCache
{
}

Any suggestion?

@dbu
Copy link
Contributor

dbu commented Jan 19, 2016

are you (or symfony) triggering loadClassCache in the kernel? this smells a lot like what we had with the cache event: https://github.com/FriendsOfSymfony/FOSHttpCache/blob/master/src/SymfonyCache/EventDispatchingHttpCache.php#L81-L82

the problem there was when the loadClassCache was also used on the console, the cache is built during warmup and then contains that file because it was not seen during the request. the cache is usually built in the first call to loadClassCache and checks what files are already loaded to not write those into the class cache file to avoid the problem you encounter.

does that help you?

@marcobarcaro
Copy link
Author

Hi,
you are right. If I clear the prod cache with --no-warmup option the error is not raise anymore, but the same configuration with SF2.7 is working, so I guess something has changed.

@dbu
Copy link
Contributor

dbu commented Jan 19, 2016 via email

@lunetics
Copy link

I got the same problem with 2.8 when warming up the cache from the console :/

@michaelperrin
Copy link

Hi @marcobarcaro , could you send your full app.php and app/console files? There might be an issue to it if you upgraded to Symfony 2.8 and didn't apply the needed changes to your bootstrap files.

@marcobarcaro
Copy link
Author

Hi,
sorry for late reply.

// app.php
use Symfony\Component\HttpFoundation\Request;

/**

  • @var Composer\Autoload\ClassLoader
    */
    $loader = require DIR.'/../app/autoload.php';
    include_once DIR.'/../app/bootstrap.php.cache';

$kernel = new AppKernel('prod', false);
$kernel->loadClassCache();
$kernel = new AppCache($kernel);

Request::enableHttpMethodParameterOverride();
$request = Request::createFromGlobals();
$response = $kernel->handle($request);
$response->send();
$kernel->terminate($request, $response);

// app/console
#!/usr/bin/env php

getParameterOption(array('--env', '-e'), getenv('SYMFONY_ENV') ?: 'dev'); $debug = getenv('SYMFONY_DEBUG') !== '0' && !$input->hasParameterOption(array('--no-debug', '')) && $env !== 'prod'; if ($debug) { Debug::enable(); } $kernel = new AppKernel($env, $debug); $application = new Application($kernel); $application->run($input);

@dbu
Copy link
Contributor

dbu commented Mar 30, 2016

@marcobarcaro i was travelling... did you find a solution meanwhile?

@pestaa
Copy link
Contributor

pestaa commented Mar 30, 2016

Hello @dbu, I found a solution.

https://gist.github.com/pestaa/d544870c8bf7c9ecdc1cb999ac918cbb

In essence, I pass an AppCache instance to the Application constructor in app/console, but a specially wrapped proxy one, so that it quacks and walks like a KernelInterface.

Edit: oh, and forgot the quick explanation: this forces the runtime to autoload all the classes used by AppCache, and therefore leaving them out from classes.php.

I consider this a good practice even with the factory default AppCache: it guarantees the contents of classes.php, whether it is warmed through console or a web entry point.

@dbu
Copy link
Contributor

dbu commented Mar 30, 2016

thanks @pestaa - ok, i can see that this can work, but its a bit of work... do you have something that triggers loadClassCache when in console mode? (or is that even triggered by default since symfony 2.8?) afaik the issue only happens when the console mode writes that cache, which "usually" it does not do. if it now does, we need to think of ways to make those classes loaded in the kernel even when not going through the cache (aka in console mode)

@pestaa
Copy link
Contributor

pestaa commented Mar 30, 2016

@dbu I see what you mean now! There is no loadClassCache from console on my end. And after further research, it doesn't seem to be the default, either. Our composer scripts also shouldn't touch anything in web/. In retrospective, I put out a candle with a fire truck. ;)

One lead worth tracking down: \Symfony\Bundle\FrameworkBundle\CacheWarmer\ClassCacheCacheWarmer::warmUp() calls \Symfony\Component\ClassLoader\ClassCollectionLoader::load(). Looks like it effectively bypasses the loadClassCache mechanism.

@dbu
Copy link
Contributor

dbu commented Mar 30, 2016

well, a fix that at least lets you continue working is good :-)

if loadClassCache is not executed, i would expect the cache to be built with the awareness of the matcher interface already being loaded. the kernel builds a list of classes and interfaces it wants in that cache (to avoid separate file access to the most common class files) but before copying those files together, it eliminates all classes that are already loaded - to prevent the problem you described. you should be able to verify this by putting a class_exists call for the classes that give errors into the console script, right after autoloading has been activated. (but that is not the fix either, what really should happen is that that cache is only built during a web request when the code path went through AppCache - afaik that is even the main reason why the cache is not built during a console command). if you have time to track down when that cache gets built, it would be great. we should work around this, or document how to avoid the problem.

@dbu
Copy link
Contributor

dbu commented Apr 5, 2016 via email

@pestaa
Copy link
Contributor

pestaa commented Apr 6, 2016

Yes, my cache muscles need a little more training. ;)

The issue is certainly caused by UserContextSubscriber loading \FOS\HttpCacheBundle\UserContext\RequestMatcher which uses the same RequestMatcherInterface used by Symfony's default request matcher.

There are 2 sane workarounds I have found:

  • Disabling kernel.class_cache.cache_warmer service by removing the definition or its tag. This is more in line with how you thought of class caching in console mode (i.e. not compiling classes.map at all).
  • Autoloading cache subscribers by initializing a throwaway AppCache instance in app/console. This is similar to what I proposed above, but without the messy proxy.

Even with all the cache subscribers enabled, only the RequestMatcherInterface tried to be loaded more than once, so in theory, autoloading it in console mode is enough, albeit not future proof (as is the case with triggering autoload CacheEvent in EventDispatchingHttpCache).

@dbu
Copy link
Contributor

dbu commented Apr 6, 2016

ah, i think you solved the mystery here! https://github.com/symfony/symfony/commits/master/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ClassCacheCacheWarmer.php was only added in symfony 2.8, it did not exist before. so indeed, with symfony 2.8+, people will need to make some extra effort to avoid problems.

could you do a pull request on the 1.3 branch of FOSHttpCacheBundle to document this in the chapter explaining how to use the symfony cache? i think we should recommend to either disable that warmer (if there is no config option for it, via a compiler pass) or by initializing AppCache even in app/console.

i agree that we don't want to start recommending manually preloading some classes - the risk is considerable that we would break that randomly by adding other classes / interfaces without being aware of it.

@stollr
Copy link

stollr commented Sep 19, 2016

I have created a CompilerPass to fix this issue for Symofny 2.8. Maybe this will help anyone ;-)

namespace Acme\Bundle\IntranetBundle\DependencyInjection;

use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\HttpKernel\DependencyInjection\Extension;
use JMS\SecurityExtraBundle\DependencyInjection\SecurityExtension;

class RemoveClassesFromCompilerMap implements CompilerPassInterface
{
    public function process(ContainerBuilder $container)
    {
        $reflProp = new \ReflectionProperty(Extension::class, 'classes');
        $reflProp->setAccessible(true);

        foreach ($container->getExtensions() as $extension) {
            if ($extension instanceof Extension) {
                $classes = $reflProp->getValue($extension);

                $cleanedClasses = array_diff($classes, $this->getClassesToRemoveFromCache());

                if (count($classes) != count($cleanedClasses)) {
                    $reflProp->setValue($extension, $cleanedClasses);

                    if ($extension instanceof SecurityExtension) {
                        $extReflProp = new \ReflectionProperty(SecurityExtension::class, 'extension');
                        $extReflProp->setAccessible(true);
                        $innerExtension = $extReflProp->getValue($extension);

                        $classes = $innerExtension->getClassesToCompile();
                        $cleanedClasses = array_diff($classes, $this->getClassesToRemoveFromCache());

                        $reflProp->setValue($innerExtension, $cleanedClasses);
                    }
                }
            }
        }
    }

    protected function getClassesToRemoveFromCache()
    {
        return [
            // The following classes are conflicting with fos/http-cache-bundle.
            // See https://github.com/FriendsOfSymfony/FOSHttpCacheBundle/issues/276
            \Symfony\Component\HttpFoundation\RequestMatcherInterface::class,
            \Symfony\Component\HttpFoundation\RequestMatcher::class,
            \Symfony\Component\Routing\Router::class,
            \Symfony\Component\Routing\Matcher\UrlMatcher::class,
            \Symfony\Component\Security\Http\AccessMap::class,
            \Symfony\Bundle\FrameworkBundle\Routing\RedirectableUrlMatcher::class,
            \Symfony\Bundle\FrameworkBundle\Routing\Router::class,
            '%router.class%',
        ];
    }
}

And in the AppBundle class you can add:

    public function build(ContainerBuilder $container)
    {
        parent::build($container);

        $container->addCompilerPass(new Acme\Bundle\IntranetBundle\DependencyInjection\RemoveClassesFromCompilerMap());
    }

@dbu
Copy link
Contributor

dbu commented Sep 19, 2016

okay. that is the other option. i feel that this has a performance impact, as the cache is to make class loading faster. also, when we change what we use in FOSHttpCache, things might break again because additional classes get used.

are you on symfony 2.8 or symfony 3? if symfony 3 keeps doing this, we should try to find a more robust solution.

@dbu
Copy link
Contributor

dbu commented Sep 19, 2016

trying to get further input in https://twitter.com/dbu/status/777833059949445120 - the fact that you need to do "something" was fixed in the documentation with #291 , online as http://foshttpcachebundle.readthedocs.io/en/latest/features/symfony-http-cache.html#subscribers

@dbu dbu closed this as completed Sep 19, 2016
@stollr
Copy link

stollr commented Sep 20, 2016

I am on 2.8 and have tried the second suggestion ("Force loading of all classes and interfaced used by the HttpCache in app/console to make the class cache omit those classes") without success. And it is reasonable that it is not working, if you look at Symfony\Component\HttpKernel\Kernel::doLoadClassCache where the loading is initiated with the $adaptive parameter set to false.

Maybe I would have tried the first suggestion, too. But I do not know how to disable class cache warming in console mode with a compiler pass without disabling it in web mode :-(

I think the performance impact of my solution should not be too bad in production environment where opcache is used. But I haven't made any benchmarks.

@dbu
Copy link
Contributor

dbu commented Sep 20, 2016 via email

@stollr
Copy link

stollr commented Sep 20, 2016

The advantage of the class cache that I see is that opcache only have to check for changes of one file (if revalidation is activated).

@dbu
Copy link
Contributor

dbu commented Sep 20, 2016 via email

@stollr
Copy link

stollr commented Sep 20, 2016

At this point in time Symfony 3.2 is no option for us (its a customer project). We just upgraded to Symfony 2.8 and rely on dependencies that are not available for 3.x :-(

@dbu
Copy link
Contributor

dbu commented Sep 20, 2016

okay. i don't have a project with symfony 3.2 at the moment either.

if you have time to help digging into this, i would appreciate if you could investigate. otherwise we will see when somebody has time to go through it.

fabpot added a commit to symfony/symfony that referenced this issue Jan 6, 2017
… (nicolas-grekas)

This PR was merged into the 3.2 branch.

Discussion
----------

[FrameworkBundle] Remove Response* from classes to compile

| Q             | A
| ------------- | ---
| Branch?       | 3.2
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #20560, symfony/demo#411, FriendsOfSymfony/FOSHttpCacheBundle#276
| License       | MIT
| Doc PR        | -

When HttpCache is used, Response is loaded first, then the kernel is booted (on cache miss), which triggers the loading of classes.php. Since 3.2 generates a context free classes.php, the Response class is now included there when it was excluded previously. And boom, "Cannot declare class Symfony\Component\HttpFoundation\Response".

Commits
-------

9ab5982 [FrameworkBundle] Remove Response* from classes to compile
@spointecker
Copy link

I can confirm the problem with symfony version 3.2.6 and php 5.6
my solution also was to disable the loadClassCache() in app.php

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

No branches or pull requests

7 participants