Skip to content

Conversation

@stollr
Copy link

@stollr stollr commented Oct 15, 2015

Symfony's class cache (cache//classes.php) compiles classes that are loaded on each request.

But when using EventDispatchingHttpCache there may occure errors, when using Symfony's HttpCache, because EventDispatchingHttpCache loads some classes (Symfony\Component\EventDispatcher\Event) individually via autoload and later when the HttpKernel is bootet in a subrequest and the class cache is loaded, PHP runs into a fatal error because of redeclaration of the class Symfony\Component\EventDispatcher\Event (as you can see in #242).

To fix this issue I have added an compiler pass to remove the mentioned classes from the cache class map before the cache is built. This only happens if cache invalidation is used (as it only happens in that case, afaik).

If there are any objections/annotations to this PR, please let me know.

@stof
Copy link
Member

stof commented Oct 15, 2015

There could be a simpler fix for that: force loading the necessary event classes in the build method of the bundle (the classes being used by the kernel wrapper), to be sure that these classes do not get included in the compiled file, as they will already have been loaded.
This is necessary because the list of classes may be dumped in cases where the event was not triggered first.

@stof
Copy link
Member

stof commented Oct 15, 2015

your own fix is very fragile:

  • any other bundle adding an event class would trigger the same issue again
  • any new event class added in FrameworkBundle would trigger it too
  • it relies on Reflection to access private state in Symfony, and there is strictly no BC guarantee on that (even in patch releases)

@stollr
Copy link
Author

stollr commented Oct 15, 2015

Thanks for your hint @stof. Can you give me a small example how to force loading the necessary event classes in the build method of the bundle?

@stollr
Copy link
Author

stollr commented Oct 16, 2015

Do you mean to simply add a new \FOS\HttpCache\SymfonyCache\CacheEvent() in the bundle's build method?

Copy link
Contributor

Choose a reason for hiding this comment

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

if the class cache is relevant, this might be an expensive thing. how do other bundles that have events solve this issue?

Copy link
Member

Choose a reason for hiding this comment

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

@dbu the issue of this bundle is that it has an event before reaching the AppKernel (in its AppCache). Other bundles don't

Copy link
Contributor

Choose a reason for hiding this comment

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

ah, thanks. yeah now i get it.

@stof
Copy link
Member

stof commented Oct 16, 2015

@Naitsirch just a class_exists call to trigger the autoloading is enough. No need to instantiate it

@dbu
Copy link
Contributor

dbu commented Oct 16, 2015

so we need a class_exists on each event class our AppCache uses in our extension class?

@stollr
Copy link
Author

stollr commented Oct 16, 2015

@dbu afaik it's enough to call class_exists('FOS\\HttpCache\\SymfonyCache\\CacheEvent'); I'll create a second PR, soon ;)

@stollr
Copy link
Author

stollr commented Oct 16, 2015

Closed in favor of #244

@stollr stollr closed this Oct 16, 2015
@stollr stollr deleted the issue242 branch October 16, 2015 13:07
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.

3 participants