-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Symlink Race Condition #242
Comments
Good catch! I just tested some code to confirm that you can just use So the following changes are all that should be necessary to improve the $cache_file_tmp = $this->cache_file.'.'.uniqid('', TRUE).'.tmp'; // Cache creation is atomic; e.g. tmp file w/ rename.
if($this->is_404 && is_file($this->cache_file_404))
if(!(symlink($this->cache_file_404, $cache_file_tmp) && rename($cache_file_tmp, $this->cache_file_404)))
throw new \exception(sprintf(__('Unable to create symlink: `%1$s` » `%2$s`. Possible permissions issue (or race condition), please check your cache directory: `%3$s`.', $this->text_domain), $this->cache_file, $this->cache_file_404, QUICK_CACHE_DIR));
else return (boolean)$this->maybe_set_debug_info($this::NC_DEBUG_1ST_TIME_404_SYMLINK); |
Yes, looks great! |
Assigning to Future Release milestone. |
@raamdev Since discovering this, I disabled 404 caching on two sites that I run. However, oddly enough this error still pops into the logs every hour or two throughout the day (with 404 caching disabled). It's puzzling me. I have no answer for it at the moment, but thought I would add that to the report here.
|
With 404 caching disabled, I also still have the I will need to do more testing and make an attempt to reproduce this elsewhere. Hopefully I will have additional information that might help with this soon. For the moment, it is a complete mystery to me. |
Very interesting! I'll do some investigation and testing of my own once I get the next release done. |
The latest release seems to have resolved this. I won't pretend to know why, but it was probably an error on my part; or perhaps just something odd in the particular branch that I had running on those sites previously. Anyway, it seems fine now. We just need to resolve the potential for a race condition when 404 caching is enabled. |
Got it. Will do! I'll add this to the Next Release milestone when I work on triaging those. :) Thanks. |
Closed by PR #72. |
I have seen this error pop up in log files recently...
Other than
symlink()
calls when 404 caching is enabled, the rest of QC writes cache files atomically; i.e. creation followed by a rename. We need to improve the way symlinks are generated also; otherwise a race condition can occur on a busy site.Referencing: https://github.com/websharks/quick-cache/blob/000000-dev/quick-cache/includes/advanced-cache.tpl.php#L914
Support Threads Referencing this Issue
The text was updated successfully, but these errors were encountered: