Doctrine 2 - ORM
  1. Doctrine 2 - ORM
  2. DDC-2210

PHP warning in ProxyFactory when renaming proxy file

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.5
    • Component/s: None
    • Labels:
      None
    • Environment:
      Windows

      Description

      Getting a PHP Warning:

      rename(**/models/Proxies_CGAF_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).

        Issue Links

          Activity

          Hide
          Benjamin Eberlei added a comment -

          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.

          Show
          Benjamin Eberlei added a comment - 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.
          Hide
          Matthieu Napoli added a comment -

          Benjamin Eberlei 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.

          Show
          Matthieu Napoli added a comment - Benjamin Eberlei 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.
          Hide
          Matthieu Napoli added a comment -

          Benjamin Eberlei 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!

          Show
          Matthieu Napoli added a comment - Benjamin Eberlei 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!
          Hide
          Matthieu Napoli added a comment -

          Benjamin Eberlei 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.

          Show
          Matthieu Napoli added a comment - Benjamin Eberlei 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.
          Hide
          Marco Pivetta added a comment -

          Matthieu Napoli 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.

          Show
          Marco Pivetta added a comment - Matthieu Napoli 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.
          Hide
          Matthieu Napoli added a comment -

          Marco Pivetta 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)

          Show
          Matthieu Napoli added a comment - Marco Pivetta 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)
          Hide
          Marco Pivetta added a comment - - edited

          Matthieu Napoli that would be very obnoxious when changing entities often. I wouldn't do that (generating only if not already available)

          Show
          Marco Pivetta added a comment - - edited Matthieu Napoli that would be very obnoxious when changing entities often. I wouldn't do that (generating only if not already available)
          Hide
          Matthieu Napoli added a comment -

          Marco Pivetta 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.

          Show
          Matthieu Napoli added a comment - Marco Pivetta 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.
          Hide
          Marco Pivetta added a comment -

          Matthieu Napoli 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.

          Show
          Marco Pivetta added a comment - Matthieu Napoli 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.
          Hide
          Matthieu Napoli added a comment -

          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.

          Show
          Matthieu Napoli added a comment - 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.
          Hide
          Marco Pivetta added a comment -

          Matthieu Napoli 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).

          Show
          Marco Pivetta added a comment - Matthieu Napoli 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).
          Hide
          Marco Pivetta added a comment -

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

          Show
          Marco Pivetta added a comment - Re-opening: the proxy factory could directly `eval()` the produced proxy code. The ProxyGenerator should no longer write the generated files to disk automatically.
          Hide
          Matthieu Napoli added a comment -
          Show
          Matthieu Napoli added a comment - I've opened a PR: https://github.com/doctrine/common/pull/269
          Hide
          Oleg Namaka added a comment -

          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.

          • In my case that is observed on a Linux machine (see http://www.doctrine-project.org/jira/browse/DDC-2613). If the rename operation is atomic then why it still happens? As I indicated in the description it's because of the way uniqid function works.
          • Is there a plan to merge the PR in this ticket? The fix in it would eliminate my issue whatsoever.
          Show
          Oleg Namaka added a comment - 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. In my case that is observed on a Linux machine (see http://www.doctrine-project.org/jira/browse/DDC-2613 ). If the rename operation is atomic then why it still happens? As I indicated in the description it's because of the way uniqid function works. Is there a plan to merge the PR in this ticket? The fix in it would eliminate my issue whatsoever.
          Hide
          Doctrine Bot added a comment -

          A related Github Pull-Request [GH-269] was closed:
          https://github.com/doctrine/common/pull/269

          Show
          Doctrine Bot added a comment - A related Github Pull-Request [GH-269] was closed: https://github.com/doctrine/common/pull/269
          Hide
          Marco Pivetta added a comment -

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

          Show
          Marco Pivetta added a comment - This issue does not need a resolution anymore, as we allow using eval() as proxy generation strategy in 2.5.

            People

            • Assignee:
              Marco Pivetta
              Reporter:
              Matthieu Napoli
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: