What scenario will reproduce the problem? Two parallel requests for the same URL gets processed. The requested URL will trigger a 'CAssetManager::publish()' method call. Now, it can happen that right before the first request will create the published asset directory, the second request will check to see if it exists too. It won't, at that stage. Now both web server processes continue execution: the first request creates the directory and second one, which saw no directory a moment ago, attempts to create the directory itself. That's a rather simple race condition, which is not handled in the code. Should it be handled? Not sure - according to the PHP manual (http://uk.php.net/manual/en/function.mkdir.php) the mkdir function will return false, and in publish() code, nobody checks the return code so it will still function ok. OTOH, why is there an 'error silence operator' ('@') in front of the chmod command in the same section of the code? I'm not sure if a PHP error will be triggered when mkdir will fail, but if so, an error silence operator is needed in front of it as well. If no error will be triggered, then the '@' can be removed from the chmod command as well.
Report relevant for Yii v1.1.7 See also a relevant forum thread that started this: http://www.yiiframework.com/forum/index.php?/topic/20908-multiple-concurrent-requests-and-the-assets-manager/
Comment #1
Posted on Jun 27, 2011 by Helpful CamelYou are right about this race condition issue. Not only this, the copy() function call faces similar issue. Unfortunately, completely solving it is not trivial and probably not very necessary in the case of asset publishing.
If the deployment script for a web application is done well, it may attempt to fetch those pages that contain such asset publishing calls and thus avoid race conditions.
Comment #2
Posted on Jun 27, 2011 by Grumpy Oxadding an 'error silence operator' - '@' prefixing the mkdir can solve it. after all, the directory is already created so this solution seems ok to me. Indeed the whole priority of this bug shouldn't be too big anyhow. pre-fetching assets during deployment is another way to bypass this issue, I agree.
Comment #3
Posted on Jun 27, 2011 by Helpful CamelAdding @ operator will make another group of people complain (our original code did have @). Anyway, this won't cause too result. When this problem happens, a very tiny amount of end users will see error pages. And if they refresh the page, it will be normal.
Comment #4
Posted on Jun 27, 2011 by Grumpy OxI agree. The only thing to left is a recommendation in the docBlock section of publish() method suggesting that its recommended on highly loaded production systems, as part of the web app publishing procedure, to publish the assets by requesting them, before the app/site goes "up". Want me to write such a suggested text?
Comment #5
Posted on Jun 27, 2011 by Helpful CamelYes, if you could. Thanks!
Comment #6
Posted on Jun 28, 2011 by Grumpy OxQiang, here's my suggested text. I recommend putting it in the docBlock of the publish() method (in fact, that's where i've written it for myself, hence the '*' marks and the newlines). Feel free to edit...:
- Note: On rare scenario, a race condition can develop that will lead to a
- one-time-manifestation of a non critical problem in the creation of the directory
- that holds the published assets. This problem can be avoided altogether by 'requesting'
- in advance all the resources that are supposed to trigger a 'publish()' call, and doing
- that in the application deployment phase, before system goes live. See more in the following
- resources:
- http://code.google.com/p/yii/issues/detail?id=2579
- http://www.yiiframework.com/forum/index.php?/topic/20908-multiple-concurrent-requests-and-the-assets-manager
Also, I would have considered putting a link to the wiki article that shed better light on the right 'handling' of the assets of a yii app, especially this part: http://www.yiiframework.com/wiki/148/understanding-assets/#hh4 But this is not entirely relevant to this bug, yet a good read for the user who's already reading the source and its documentation.
Boaz.
Comment #7
Posted on Jun 28, 2011 by Helpful CamelThanks. Added.
Status: WontFix
Labels:
Priority-Medium
Milestone-1.1.8