Skip to content

Loading…

DDC-2210: PHP warning in ProxyFactory when renaming proxy file #2907

Closed
doctrinebot opened this Issue · 18 comments

2 participants

@doctrinebot

Jira issue originally created by user mnapoli:

Getting a PHP Warning:

rename(****/models/Proxies\__CG__AF_Model_Component_Group.php.50d2dd2c079bb9.35271255,****/models/Proxies\*_CG__AF_Model_Component*Group.php):

in ProxyFactory line 194.

I haven't more information in the warning.

This is the moment when the ProxyFactory writes the proxy to a temporary file and then tries to rename the temp file to the correct file.

This warning appears randomly, but mostly on pages with lots of concurrent AJAX requests. I guess this happens because several requests try to write the proxy file at the same time. I get this warning but the app works fine.

This happens in dev environment, on a Windows machine.

I don't know why rename generates a warning, it should just return false... The doc doesn't say anything about warnings (except for long file names, but I checked even with the full path this is around 135 characters, not 255).

@doctrinebot

Comment created by @beberlei:

Thats why you shouldn't generate proxies at runtime. The problem happens on windows, because the atomic rename operation doesn't work as perfectly there as on linux.

We cannot fix this in Doctrine.

@doctrinebot

Comment created by mnapoli:

[~beberlei] What do you mean "you shouldn't generate proxies at runtime"? I'm not in production, this is in dev. And I'm using the default configuration.

What I don't understand is why will Doctrine regenerate proxies on every request? The warning is reproductible, and even when no PHP entity has been touched.

@doctrinebot

Comment created by mnapoli:

[~beberlei] To simplify my previous message (I don't want to bury you under questions) I'll sum it up like that:

What can I do?

Thanks!

@doctrinebot

Comment created by mnapoli:

[~beberlei] ping: what can be done?

Can we suppress the error with @rename($tmpFileName, $fileName); ?

https://github.com/doctrine/common/blob/master/lib/Doctrine/Common/Proxy/ProxyGenerator.php#L287

I can make a PR if you think that's a valid solution.

@doctrinebot

Comment created by @ocramius:

[~matthieu] no, if you have warnings, please disable them via ini setting. With error suppression there, we may have further problems identifying more serious issues.

About proxy generation: that happens EVERY time in dev environments. Generate them once and disable it afterwards.

@doctrinebot

Comment created by mnapoli:

[~ocramius] OK I can disable the auto generation then (I'll have to remember to regenerate them when I edit the model).

Thanks for the feedback

Is that possible to make those proxies generate only if the entity file has been modified since the last generation? (only asking if can and should be done, I can look for implementing it myself if that's the case)

@doctrinebot

Comment created by @ocramius:

[~mnapoli] that would be very obnoxious when changing entities often. I wouldn't do that (generating only if not already available)

@doctrinebot

Comment created by mnapoli:

[~ocramius] Yes but for now they are regenerated at every request when in dev mode (at least with the default configuration http://docs.doctrine-project.org/projects/doctrine-orm/en/latest/reference/configuration.html#obtaining-an-entitymanager)

Which one is worse: generating every proxy class at every request, or generate only those which changed (in dev environment of course, not prod)?

If neither of these options are good (i.e. auto generation should be disabled), I don't understand why the docs say to enable auto generation when in dev environment.

@doctrinebot

Comment created by @ocramius:

[~mnapoli] that's because in dev environments you shouldn't care about that one exception (usually happens when you got concurrent requests).

It is worse to generate only on changes: that's a lot of additional checks, variables to keep in memory and additional logic that is not needed.

Let's keep it as it is (generating at each request) for dev environments: works fine :)

Another (eventual) solution for dev environments would be not to write the proxy file, but to eval it.

@doctrinebot

Comment created by mnapoli:

eval it would be a good solution IMO, no more "woops the directory is not writable" and it's more neutral for the user filesystem (but not as easy to debug). But OK, I see what you mean, it works let's keep it that way.

Actually the problem on my setup is that PHP errors are turned into exceptions, so on an (poorly designed) AJAX treeview (lots of nodes to load => lots of requests), I end up with some nodes not loaded because of the exception. And it feels weird to either silently log all PHP warnings or silently ignore the specific warning for the rename.

@doctrinebot

Comment created by @ocramius:

[~mnapoli] I'd go with eval then. Needs refactoring of the abstract proxy factory and of the proxy generator (proxy generator should no longer write files).

@doctrinebot

Comment created by @ocramius:

Re-opening: the proxy factory could directly eval() the produced proxy code. The ProxyGenerator should no longer write the generated files to disk automatically.

@doctrinebot

Comment created by mnapoli:

I've opened a PR: doctrine/common#269

@doctrinebot

Comment created by deatheriam:

Thats why you shouldn't generate proxies at runtime. The problem happens on windows, because the atomic rename operation doesn't work as perfectly there as on linux.

@doctrinebot

Comment created by @doctrinebot:

A related Github Pull-Request [GH-269] was closed:
doctrine/common#269

@doctrinebot

Comment created by @ocramius:

This issue does not need a resolution anymore, as we allow using eval() as proxy generation strategy in 2.5.

@doctrinebot

Issue was closed with resolution "Fixed"

@Ocramius Ocramius was assigned by doctrinebot
@doctrinebot doctrinebot added this to the 2.5 milestone
@doctrinebot doctrinebot closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.