Fix ZF-11921: Race condition in plugin loader include file cache

[nicolas-grekas]

This fixes the race condition described at:
http://framework.zend.com/issues/browse/ZF-11921

For the race to disappear each and every line is made completely independent from the others. This is the reason why every line includes its own php open and close tag, and no file_get_content() is made, and the file is opened in append only mode.

To prevent the insertion of duplicates lines, a fast non blocking lock is made and kept until the end of the request, thus the static $h.

Tested on a highly concurrent server without a problem.

[weierophinney]

Can you name this more sematically, please? $handle, , $cacheFile, etc...

[simonrjones]

We just hit this issue on a recent deployment. Although it happens very rarely, it is a pretty serious issue when it does occur since it spits out all the included files to the top of the HTML page.

It would be good to get this fix into ZF if the code change is OK. Is ZF1 still getting security fixes for things like this? Do you need any assistance testing this to get it approved?

[mhujer]

@nicolas-grekas it would be great, if you can rebase it on latest master (and eventually squash it). Travis passes for master, so it would be easy to see, if this does not break anything.

[nicolas-grekas]

Rebase done, tests pass!

[froschdesign]

@nicolas-grekas
Sorry, but I can not find a signed CLA. Please send me your e-mail address and I'll check again.

[nicolas-grekas]

firstname.lastname at gmail
com

[froschdesign]

Hi Nicolas,
I can not find any signed CLA under your e-mail address. Sorry!
Have you ever sent a CLA?

[nicolas-grekas]

Never, I don't know anything about your process, I just do PHP ;)

[simonrjones]

Is this update stuck because of the CLA? If so, I have signed one for ZF1 if it's any help...

[akrabat]

Ideally, we need @nicolas-grekas to sign the CLA (PDF download) and email it to [email protected].

[nicolas-grekas]

I'll do that. Thanks for the links @akrabat

[akrabat]

@nicolas-grekas Did you get the CLA sent?

[nicolas-grekas]

Just now!

[akrabat]

Thanks!

[kevinwritescode]

I just wanted to note that this change caused our zend caching to no longer work since we were externally creating the cache file with <?php appended to it before calling Zend_Loader_PluginLoader::setIncludeFileCache($cachePath);. This additional <?php would then trigger this error:

Parse error: syntax error, unexpected '<' in .../zendCache.php on line 2

I don't know if you'd consider that breaking backwards compatibility but I wanted to make mention of it in case anyone else has a similar issue. I can't comment as to why our legacy code was prepending <?php outside of setIncludeFileCache() but that's just how it was setup.

[mhawk0]

The plugin cache does not check for duplicates, but simply adding new require_onces at the end.
Before the change there was at least a check for

if (!strstr($file, $incFile)) {

but it's gone now, and it became useless as the file grows to hundreds of megabytes or more!

[froschdesign]

@marcing or @mhawk0
Please open a new bug report. Thanks!

[mhawk0]

Sorry for an alert too early.
Problem was on my side, as I misinterpreted the docs.
I thought that

    Zend_Loader_PluginLoader::setIncludeFileCache('a path to file');

is enough, but the include file cache was supposed to be manually included first:

    $classFileIncCache = APPLICATION_PATH . '/../data/pluginLoaderCache.php';
    if (file_exists($classFileIncCache)) {
        include_once $classFileIncCache;
    }

I assumed that the file is included automatically in Zend_Loader_PluginLoader::setIncludeFileCache() method.

After all, the cache never worked properly for us, as the include cache file was only written but not read. Before the change duplicates were not added because the whole file was read by file_get_contents and checked. After the change all includes were just appended to the end of file.