Doctrine 2 - ORM
  1. Doctrine 2 - ORM
  2. DDC-716

Proxy autogeneration fails with concurrent requests

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0-BETA2
    • Fix Version/s: 2.0-BETA3
    • Component/s: ORM
    • Security Level: All
    • Labels:
      None
    • Environment:
      Debian 5.0.5, Apache 2.2.9, PHP 5.3.2

      Description

      When doing concurrent requests with autogeneration of proxies enabled, the proxy file does not exist when ProxyFactory tries to use it:

      Doctrine/ORM/Proxy/ProxyFactory.php(92): spl_autoload_call('MyClassProxy'))
      

      I think this is because file_put_contents is not atomic.

        Activity

        Hide
        Jaka Jancar added a comment -

        The following fixes it on Linux, but I think will not work in Windows (iirc, rename() on Windows won't work if the dest file already exists, much less atomically):

        
        --- Proxy/ProxyFactory.php	(revision 2)
        +++ Proxy/ProxyFactory.php	(working copy)
        @@ -144,7 +144,9 @@
         
                 $file = str_replace($placeholders, $replacements, $file);
         
        -        file_put_contents($fileName, $file);
        +        $tmpFileName = $fileName.'-'.uniqid('', true);;
        +        file_put_contents($tmpFileName, $file);
        +        rename($tmpFileName, $fileName);
             }
         
             /**
        
        Show
        Jaka Jancar added a comment - The following fixes it on Linux, but I think will not work in Windows (iirc, rename() on Windows won't work if the dest file already exists, much less atomically): --- Proxy/ProxyFactory.php (revision 2) +++ Proxy/ProxyFactory.php (working copy) @@ -144,7 +144,9 @@ $file = str_replace($placeholders, $replacements, $file); - file_put_contents($fileName, $file); + $tmpFileName = $fileName.'-'.uniqid('', true );; + file_put_contents($tmpFileName, $file); + rename($tmpFileName, $fileName); } /**
        Hide
        Jaka Jancar added a comment - - edited

        I don't even know why this is needed. Can't the file just be returned as string and eval()'d, instead of being written to a file and then require()'d?

        Show
        Jaka Jancar added a comment - - edited I don't even know why this is needed. Can't the file just be returned as string and eval()'d, instead of being written to a file and then require()'d?
        Hide
        Benjamin Eberlei added a comment -

        we should just change file_put_Contents into:

        file_put_contents($fileName, $file, LOCK_EX);
        
        Show
        Benjamin Eberlei added a comment - we should just change file_put_Contents into: file_put_contents($fileName, $file, LOCK_EX);
        Hide
        Benjamin Eberlei added a comment -

        Setting the LOCK_EX constant now, this should solve the issue.

        However in high concurrency scenarios the "autoGenerateProxyClasses" flag should always be FALSE and the proxies be generated during build-time.

        Show
        Benjamin Eberlei added a comment - Setting the LOCK_EX constant now, this should solve the issue. However in high concurrency scenarios the "autoGenerateProxyClasses" flag should always be FALSE and the proxies be generated during build-time.
        Hide
        Jaka Jancar added a comment -

        LOCK_EX doesn't fix it for me. Apparently it's still possible that the file doesn't exist:

        E_WARNING (2): include(MyProxies/MyClassProxy.php) [<a href='function.include'>function.include</a>]: failed to open stream: No such file or directory
        

        Please note that my above uniqid+rename suggestion only works if more_entropy is true, if you decide to add it.

        However, I'd much prefer having an option of just not using these files at all. I've created an Improvement ticket DDC-717 for this.

        Show
        Jaka Jancar added a comment - LOCK_EX doesn't fix it for me. Apparently it's still possible that the file doesn't exist: E_WARNING (2): include(MyProxies/MyClassProxy.php) [<a href='function.include'>function.include</a>]: failed to open stream: No such file or directory Please note that my above uniqid+rename suggestion only works if more_entropy is true, if you decide to add it. However, I'd much prefer having an option of just not using these files at all. I've created an Improvement ticket DDC-717 for this.
        Hide
        Benjamin Eberlei added a comment -

        i now underestand what you are doing wrong.

        if you set autogenerate = false you have to call doctrine orm:generate-proxies

        Show
        Benjamin Eberlei added a comment - i now underestand what you are doing wrong. if you set autogenerate = false you have to call doctrine orm:generate-proxies
        Hide
        Jaka Jancar added a comment -

        I'm not setting it to false!

        > When doing concurrent requests with autogeneration of proxies enabled

        Show
        Jaka Jancar added a comment - I'm not setting it to false! > When doing concurrent requests with autogeneration of proxies enabled
        Hide
        Benjamin Eberlei added a comment -

        ah ok, eval is just a workaround.

        Show
        Benjamin Eberlei added a comment - ah ok, eval is just a workaround.
        Hide
        Benjamin Eberlei added a comment -

        Closed again, See DDC-717 for a timetable of a fix using auto-generate = true in production.

        Show
        Benjamin Eberlei added a comment - Closed again, See DDC-717 for a timetable of a fix using auto-generate = true in production.

          People

          • Assignee:
            Benjamin Eberlei
            Reporter:
            Jaka Jancar
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: