Doctrine 2 - ORM

Proxy autogeneration fails with concurrent requests

Details

  • Type: Bug Bug
  • Status: Resolved 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

Ascending order - Click to sort in descending order
Hide
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: