Make WordPress Core

Opened 17 years ago

Closed 16 years ago

#3907 closed defect (bug) (fixed)

WP reverts to default theme on file access collision

Reported by: _ck_'s profile _ck_ Owned by:
Milestone: 2.5 Priority: low
Severity: major Version: 2.2
Component: Template Keywords: has-patch
Focuses: Cc:

Description

There is still no fix addressed to the problem on heavily accessed systems where the theme can revert back to default.

This apparently happens if the "file_exists" test in theme.php fails under any of a few different functions? PHP manual shows that "file_exists" can fail if a theme file is being replaced at the time or certain conditions occur under php safe mode.

One way to reproduce this bug consistently is to upload "style.css" repeatedly while a site is active.

I'd like to suggest that "file_exists" be replaced with a custom function that does at least a few retries over an adjustable time (a second or so) before completely giving up and reverting.

Attachments (1)

3907.diff (383 bytes) - added by lloydbudd 16 years ago.

Download all attachments as: .zip

Change History (14)

#1 @_ck_
17 years ago

Untested suggestion, some code borrowed from php.net

function wp_file_exists($file) {
$start = gettimeofday(); 
     while (!file_exists(trim($file, " '\""))) {
       $stop = gettimeofday();
       if ( 1000000 * ($stop['sec'] - $start['sec']) + stop['usec']   - $start['usec'] > 500000) {
           return file_exists(trim($file, " '\"")); }
     } 
return true;
}

#2 @markjaquith
17 years ago

Aha. I've heard of the symptom, but had no idea of the root cause. Thanks for the report.

Maybe we could do this:

  1. read the active theme name from the options table
  2. update the file
  3. check the active theme name from the options table, and revert if it has changed since #1

#3 @foolswisdom
17 years ago

  • Milestone changed from 2.2 to 2.3
  • Version set to 2.2

#4 @f00f
17 years ago

  • Severity changed from normal to major

markjaquith: What do you mean by 'update the file'?

The scenario (if I understood correctly) is as follows:

While you are uploading a new stylesheet (or other theme file) via FTP a user visits your site. Then file_exists() could fail because the file is actually opened by another process than the web server.

As a result the current theme gets permanently reset to 'default' in validate_current_theme().

IMHO, one second is to long for a timeout.

Another solution might be to check if the template folder exists and assume that index.php and style.css exist in it.

#5 @ryan
17 years ago

  • Milestone changed from 2.3 to 2.4

#6 @Denis-de-Bernardy
17 years ago

  • Milestone changed from 2.4 to 2.3

Could we get this into 2.3? I've seen this happen a couple times, and it's rather annoying when it comes up. Changing the code to check for the template folder would work imho, and the change is rather trivial.

D.

#7 @foolswisdom
17 years ago

  • Milestone changed from 2.3 to 2.5

Without a patch attached no. Without testing of the solution, no.

Moving to milestone 2.5 pending a tested patch.

#8 @markjaquith
16 years ago

  • Keywords needs-patch added

I've had more reports of this happening.

I like the idea of waiting before deciding the file is not there. It should be a very rare event, so it's okay if the user that triggers it has to wait a few seconds while we make sure the selected theme really is gone.

#10 @ryan
16 years ago

How about we just lose this? We reset broken themes when visiting the Presentation admin page. Let's call that good enough.

#11 @lloydbudd
16 years ago

+1 to removing the call validate_current_theme() from wp-settings.php

file_exists can fails (returns false) when there is file contention. I've been working with a friend who supports a number of busy blogs and on two different hosting environments he has issue with regular resets -- and by regular, I don't mean I can see a pattern to the time interval, sometimes it can be a few minutes a part, sometimes a few hours.

validate_current_theme with its 2 file checks is useful for basic sanity when going to the Presentation tab, and it would be great if it did more checking, but a synchronous wp_file_exists on every load of WP seems awkward, and won't detect most problems with a theme.

@lloydbudd
16 years ago

#12 @lloydbudd
16 years ago

  • Keywords has-patch added; needs-patch removed
  • Milestone changed from 2.5 to 2.4

#13 @ryan
16 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [6325]) Skip theme validation during normal blog page loads. fixes #3907

Note: See TracTickets for help on using tickets.