Themes disabled during update

subscribe

I'm experiencing this as well, but not only with themes - it's happening with modules also, so I'm pretty sure it's not being caused by the theme or theme settings. I've been trying to recreate the problem, but I can't tell when it's being disabled or why because it's very sporadic.

I've also noticed that when I make a change to post information visibility, by node, the changes are not reflected unless I go to the theme specific settings page, and resubmit the form, which is very odd. This may or may not be related, but I figured it was worth mentioning here.

I will keep trying to figure it out...

I am also experiencing this same problem with Drupal 6.4.

Same problem here -- it may be coincidence but it often seems to happen after updating a module.

I too am experiencing this particular problem. It's driving me crazy!

After doing some testing, it appears that the problem lies in the update.php file. If you run that, it will disable all your themes.

thx

This issue also appears to be in 7.x.

I've attached an updated patch against the latest CVS for both 6 and 7, and changed the component to the update system as this seems to more accurately reflect the cause, as the function appears to work fine outside of update.php.

See also #147000: Rewrite module_rebuild_cache() and system_theme_data(). Calling system_theme data will do a complete delete and insert for all themes in the system table, which creates a race condition and creates the same problem that has been described here.

Steps to reproduce for anyone trying to verify/test this:

1. Go to Site Building -> Themes
2. Make sure that one or more themes are enabled
3. visit update.php (You don't have to run any actions, just going to the page will trigger the bug)
4. Go back to Site Building -> Themes

Without the patch, enabled themes will no longer be enabled. With it, they should still be enabled.

Confirmed by visiting update.php with and without site in maintenance mode.

Actually, this should be fixed just by changing the call from system_theme_data to _system_theme_data (which is supposed to be used since we don't want to hit the database) instead of adding an unneeded parameter to system_theme_data. I confirmed this patch fixes the problem on a patched HEAD by visiting update.php. My themes were no longer disabled and it did not change the update process.

Nice. :) I didn't even try that, or think that it would work, but I suppose it makes sense. +1 from me.
(I had trouble applying the D7 patch, so I re-rolled it).

Patch in #17 should apply cleanly. It passed testing.d.org: http://testing.drupal.org/node/15099

Clarifying title

@ Dave Reid in #19
The D6 patch applied cleanly for me, and the D7 one applied, but spat out a whole bunch of other stuff I haven't seen before, and then patch crashed. It was most likely something quirky on my end. The patch does look fine though. I can't explain it.

To reproduce, enable a couple themes, go to update.php (don't need to do anything else), then revisit the themes page. Your themes are no longer enabled.

After patch, checkboxes stay checked.

Committed, thanks!

Moving back to 6.x.

subscribe

no success for me.

Unfortunately, applying the patch, change to $themes = _system_theme_data(); in update.php does NOT do any different. The Enabled check box is not checked anymore, but Default yes. So the site is still running with the theme.

ealier without realizing this, whenever configuring blocks, I get Access Denied , because no theme is enabled. err ... iam lucky I realize the issues and found this thread.

I can confirm, the D6 patch does not work.

Note: to reproduce the error, it is not enough to visit the update.php page, you need to click on the update button..

I'll look into this shortly...

Ok, wow. I hate these functions so much. Finally figured out what is going wrong. I'll try to explain as clear as I can.

• On an update page, the first relevant call is list_themes()
• list_themes() calls _system_theme_data() and then uses:
  

      $themes = _system_theme_data();
      foreach ($themes as $theme) {
        ...
        // Status is normally retrieved from the database. Add zero values when
        // read from the installation directory to prevent notices.
        if (!isset($theme->status)) {
          $theme->status = 0;
        }
        ..
      }
  

• When list_themes() modifies that data, those modifications are also saved to the static data variable in _system_theme_data since it is automatically returned from _system_theme_data by reference.
• Now when whenever we call system_theme_data (which calls _system_theme_data within it), all the $theme->status variables are set to 0.
• system_theme_data calls system_get_files_database on the data from _system_theme_data.
• Since the status key is set, the system_get_files_database function will not override the status key with the current status from the {system} table.
• This data (including every theme's status->0) is now saved to the {system} table within system_theme_data, disabling all themes.

Where can we go from here to solve this?
1. We implement a $reset = TRUE parameter as was proposed in #13 for _system_theme_data so that system_theme_data will always get a fresh copy and not the saved static copy that could have been modified. Any calls to system_theme_data should result in the correct data.
2. Change system_get_files_database so that if a column in the {system} table is different from a key in the module/theme data, grab the value from the {system} table record. Any calls to system_theme_data should result in the correct data.
2. Make list_themes work a little smarter and not have to set status = 0 for all the themes.

I'll work on a patch...

I hate these system_theme_data functions. A lot.

subscribe

I must admit, I didn't test the latest patch all the way through an update, I did however test the one in #13 all the way through, so I know that that solution will work, however I do not really like it, as caching is a good thing. So that rules out option number 1.

Option 3 seems impossible to achieve. At least I can't find away.

So this leaves option 2. In this light, I've put together a nice simple patch that I've tested and seems to work well.

Let me know what people think.

Moving back to 7.x

Re #29, this is more what I was thinking:

     if (isset($files[$file->name]) && is_object($files[$file->name])) {
       $file->old_filename = $file->filename;
       foreach ($file as $key => $value) {
-        if (!isset($files[$file->name]) || !isset($files[$file->name]->$key)) {
+        if (!isset($files[$file->name]->$key) || $files[$file->name]->$key != $value) {
           $files[$file->name]->$key = $value;
         }
       }

The check for !isset($files[$file->name]) is removed since it will never hit because we check isset($files[$file->name]) three lines above it.

To test this on your own:

$changed_themes = _system_theme_data();
foreach ($changed_themes as $a_theme) {
  $a_theme->foobar = 'ferzle';
  $a_theme->status = 0;
}
$unchanged_themes = _system_theme_data();
var_export($unchanged_themes);

You will see that the $unchanged_themes variable has the changes made in the $changed_themes variable. A solution to fix this issue should not let that happen.

I believe I've found the problem, although I'm not sure on the best way to fix it.

From a comment I found at php.net

On array copying a deep copy is done of elements except those elements which are references, in which case the reference is maintained. This is a very important thing to understand if you intend on mixing references and recursive arrays.

As each of the themes within the returned array is an object, a reference to the object is returned, causing any changes done, to actually affect the static $themes_info in _system_theme_data.

Does anyone have any suggestions on how we could cleanly make sure the returned objects in the array are copies? It would seem that looping might be the only way to achieve it.

Hi all,

Here's a totally different approach to the problem.

Everyone's concentrating on the fact that status gets set to 0 for these themes.

Is anyone questioning why it gets written to the database?

It's a cached copy, isn't it? Why are we doing database inserts of (arguably incorrect) cached copies which we (arguably) may not have to?

update.php calls update_check_incompatibility() which calls system_theme_data() which unconditionally always writes the theme info structure to the database, including incorrect status values.

Can we add a param $update_db = FALSE to system_theme_data() so it doesn't write to the database when it is called by update.php?

[EDIT]

Hmmm... update.php also calls drupal_flush_all_caches() which calls system_theme_data(), so that's another write to the database which needs to be neutered.

@35. The problem is still any call to either _system_theme_data or system_theme_data will return the invalid status=0. So if were trying to list themes, show current theme, do anything with themes, we're now dealing with invalid data. True, this should also avoid loading back to the database. I'm going to work on an end-all patch tonight for this issue.

@36:

Ah okay, fair enough. I didn't think of it that way.

You (and others) have spent more time on the subtleties, so I will defer.

Thank you.

Ok after a metric-crap-load of testing, I can really only see two approaches to fix this problem of the themes array being deep copied.

1. Perform a shallow copy on the themes array in _system_theme_data so that modifications to the themes array don't modify the static variable.
2. Implement a $reset parameter in _system_theme_data that will not use/trust the cached static variable in the cases that data will be written to the database.

Personally, I prefer approach #2, but let's get some more input.

Good work on sorting this all out Dave.

I'm impartial to which method is ultimately chosen, so long as it works.

Here's what I think the pro's and con's of each method are:

Method 1: Implicit Shallow Copy

Pros:

1. No change to current interface
2. users of system_theme_data do not need to consider if they need a 'clean' copy, and they don't need to worry about the case where another function/module has altered the data when perhaps it should not have.

Cons:

1. Potential for higher memory usage, and longer execution times.

Method 2: Explicit reset

Pros:

1. Caller is guaranteed to get a clean copy of the data when they request it.
2. Caller can use the static value if they don't care about the integrity of the data.

Cons:

1. To guarantee integrity, callers will have to use the reset.
2. Forces the caller to use reset when they are saving information to the database.
3. More potential for subtle bugs. (Easily fixed by adding the $reset = TRUE, but only if found.)

Given all that, I would say that I would lean more towards number 1. These are my opinions, and I'm happy to have them debated and proven wrong. :) Just looking for the best solution.

Congratulations, Dave Reid.

Tested patch #2 from comment 38 and it applies fine to Drupal 6 with a fair amount of fuzz. It fixes the problem of no theme enabled for a non-default installation profile.

I would say this is ready to go in, but I know it has to be tested and applied against Drupal 7 first.

@cdale, I think calling reset when data is involved is best– this really applies to installation and updating, so it's Drupal core's problem, correct?

I fear this will go back and forth a few more times but, already, thanks very much for work, in particular by Dave.

^{benjamin, Agaric Design Collective}

I think with #38 patch 2, you did not solved your test case in #32. The question: is it necessary to pass that test or that test is wrong? What is the expected functionality for _system_theme_data() function?

Also patch 2 is an API change. So to get accepted for D6, then it should be verified that it is the best solution.

I would like to see this bug fixed before the next release..

I've been using patch #1 and it works great. No issues what so ever.

Having looked at patch #2, if that is the method that people prefer, then I think some other changes might be required. For example, going though core, the majority of calls are to system_theme_data, not _system_theme_data, and the calls to _system_theme_data are only on maintenance pages, or pages that are not allowed to hit the db. So with patch #2, what is the purpose of leaving $themes static, as from what I can tell, $themes will be rebuilt every time they are requested under normal usage as $reset will always be true. Is this ideal?

I'm not meaning to sound overly critical, I think Dave Reid has done a great job, I'm just trying to find the best solution to a problem, and I personally feel that patch #2 is not it given the methods that system_theme_data is used throughout core.

Is there any particular reason people prefer patch #2 over patch #1?

Perhaps another approach could be to use the clone method, but only when the caller knows it will be modifying the data in a way that other calls would not desire.

I've attached a patch that I think demonstrates what I mean by that approach. The patch also reverts the change committed in #22.

Does this seem like a better idea to most?

Testing this faster in current HEAD:

UPDATE system SET schema_version = '7010' WHERE name = 'system';

Afterwards, execute update.php, update, and see no theme enabled.

As for the patches, I tend to the $reset variant. Analyzing now.

Also, in D7 use "clone", in D6 we use drupal_clone() to clone data structures.

The solution is actually trivial:

In maintenance mode, do not update theme data in the database at all.

update.php's update_check_incompatibility() already cares for disabling modules and themes that are incompatible with the current version of core, so we can safely avoid this database update.

This patch also reverts the original patch the has been committed. So after applying this patch, just visiting update.php is sufficient to see that themes are no longer disabled.

Same patch for 6.x.

Confirmed the bug and that the D7 patch fixes it. RTBC.

I think in D6 the maintenance_mode is not set at update..

this code is used in D6 for the update script check:

strstr($_SERVER['SCRIPT_NAME'], 'update.php')

cross post

@Pasqualle: It is, see line 22 of update.php.

+1 from me on sun's patch. It's so simple, and works beautifully.

It still does not pass the use case in #32, but I feel that this may be outside the scope of this issue now, as this is only a problem when running in maintenance mode.

I have no clue why the D7 patch was reported to fail. Detailed results page even mentions that it was not tested in testbed at all. Patch still applies cleanly.

Why do we call system_theme_data() to begin with?

ok, after analyzing the whole process once more:

• update.php invokes system_theme_data() to gather all themes and test them for Drupal core compatibility.
• update.php also invokes _drupal_maintenance_theme(), which invokes list_themes() and _system_theme_data(). Note that those calls do not cause this bug, because _system_theme_data() does not alter the database. Only system_theme_data() alters the database.
• However, #35: When update.php finishes, we call drupal_flush_all_caches(), which invokes system_theme_data().

This effectively means that we could probably place the same or similar code into drupal_flush_all_caches(), and also ensure that we ALWAYS invoke _system_theme_data() in update.php and theme.maintenance.inc... - actually no, since we still want to flush all theme data when drupal_flush_all_caches() is invoked in maintenance mode.

So the proper place to prevent a database update with malformed values is in system_theme_data(). Additionally, I think we should avoid invocations of special maintenance mode functions where possible and leave it to the subsystem to properly handle the maintenance mode case. That helps in demystifying our install/update/maintenance mode process.

subscribing

Will the patches fix this problem on an existing DB i.e. on that is already getting the problem? Or do I need to do something manually to clean out dirty data?

@46: Don't use this patch!!

Sorry, Sun, I'm not rubbishing your patch. I'm only shouting so people don't use it.

The patch in #46 fixes update.php, but unfortunately, it breaks a fresh install of drupal.

@53: I concur with this line of questioning.

Going along the lines of reasoning of #35 and #53 ... why does drupal_flush_all_caches() want to call a function that writes to the database?

This just seems wrong (IMO). Flushing implies reading new data from the database, not writing data to the database (regardless of whether said data is correct or not).

BTW, the call to system_theme_data() in drupal_flush_all_caches() was added in includes/common.inc 1.756.2.27, between Drupal 6.2 and 6.3, for #239958: Clearing cache does not immediately reload theme's .info file

Reverting back to second option mentioned in my last summary. Works from my own testing.

To test this:

1) Go to admin/build/themes and ensure that at least one theme is enabled.
2) Execute the query in #44.
3) Visit update.php, but do not execute updates.
4) Go to admin/build/themes and confirm that the theme(s) is/are still enabled.
5) Visit update.php and execute updates.
6) Go to admin/build/themes and confirm that the theme(s) is/are still enabled.
7) Nuke your Drupal HEAD test site.
8) Install from scratch and confirm that Garland theme is enabled in admin/build/themes.

And, yes, this adds cruft to drupal_flush_all_caches(). However, I do not think that duplicating that function's contents into update.php would be a good idea.

I got a bit confused about versions of core, so just clarifying ...

My comment #57 refers to Drupal 6.6. Sun's patch #60 refers to Drupal 7.x.

Some testing:

I tried the patches from #46 and #60 together against Drupal 6.6 (I backported #60 by hand myself, as it's not meant for 6.6).

Result: update.php no longer disables theme's, but when installing a fresh installation, the theme is not enabled. Breakage.

Forget previous patches. Only apply #60. It's supposed to be applied on D7/HEAD, since D6 needs an additional tweak to get this working. However, D6 will be fixed after this patch hit D7.

sub

subscribe

subscribe

This bug has ugly side effects, at least in D6. E.g. that custom blocks created by users are not written to the blocks table in block_add_block_form_submit (as it checks for enabled themes) but only in _block_rehash. This causes the block title which the user entered in the form to be lost.
And it makes hook_block('save') in any custom module completely useless for user-created blocks because the block is not yet written.

Hoping for a soon fix in D6...

subscribe

The side effects are for all functions that lists modules. For example themes are not longer tracked by update status. The System info module gives out an empty theme list, etc.

I believe that Drupal 7 will be fixed before D6, see comment #62.

Failed due to #74645: modify file_scan_directory to include a regex for the nomask.. Setting back to code needs review.

subscribe

subscribing

This is an easy patch to test, why not try it and report back? Might even get fixed that way...

I would like to do that but I don't have D7, only D6 and sun said that there was some additional patch needed which we get only after D7 has been fixed :-(

I can report that it works fine for my installed D6, knowing that it is supposed to break a fresh installation.

webchick, I think this should be a 7-0-UNSTABLE-4 blocker to agree on a solution and get this fixed and then backported.

This should also become a blocker for a D6.7 release.

it does not blocked D6.6 so I don't see the point why it would block the next release..
It's just a critical D6 bug as any others

Hmm, most of the post in the list you linked to are not really critical bugs as they don't influence any other stuff. And many of the post end with "Oh, found the bug, was my fault" or sth.

This bug influences a lot of other modules, like stopping custom blocks and hook_block from working correctly (just to name the side effect that affects me), and this is bit more severe than most of the other "critical" bugs in your link...

subscribe

I have noticed that theme gets disable after cron runs too! Is this related to this issue or separate (this is for drupal 6 but thought it might give you more information)

Forcing re-test of last patch.

If your system has "no theme enabled", custom blocks (and Menu block module blocks) can't save their settings because those settings are saver per-enabled-theme. :-p Pretty critical. But debating whether this is a 6.7 blocker is pointless since a security bug will always force a new 6.x release regardless of any pending critical bugs.

Too much subscribing (we really need to fix the comment-to-get-email-updates "bug") and not enough testing!

Before I applied the patch, I created a bogus book_update_6001() to test. I can confirm that running update.php will nix any enabled themes. I can also confirm that running cron.php does not disable themes, spopovits. This is with all core modules enabled and no contrib modules.

After I applied the patch, I created a bogus book_update_6002() to test. And I can confirm the patch fixes the issue.

John: You don't need to add any bogus update hooks... simply run update.php to clear caches and all themes are disabled.

subscribe

subscribe

subscribe

Committed this to HEAD. Thanks!

Moving back to 6.x.

Trivial re-roll of sun’s patch. Should be RTBC, but I’ll let someone else double check.

For D6, we also need at least the changes from the patch in #18.

Done.

The patch in #90 applied cleanly for me on two different sites running 6.6 and appears to fix the problem.

I'd say RTBC then.

subscribe, patch applied for live site which often breaks

I've open #344625: Themes disabled on cron because this patch work only for update.php which is deleted on site but after 7 hours site "forget" about custom theme again

+1 The patch in comment #90 works fine for me for both invoking update.php and a fresh install.

Committed to 6.x, thanks!

I'm afraid you fixed only symptoms of the bug with _system_theme_data().
See #632080: _system_theme_data() should clone the returned objects. .

This issue's reported problem might be fixed, but the underlying technical problem is still waiting for the next chance to make trouble. I mark this as "needs work", because I would like the attention of those people who worked on this issue.

----

A full explanation is in the issue linked to in #632080: _system_theme_data() should clone the returned objects., but here the short version:

The cache in _system_theme_data() is designed to remember the results from a system directory scan. This information does usually not change within one request, so it should not be a problem.

system_theme_data() brings in additional information from system_get_files_database(), which is not supposed to be cached. We expect fresh information if we call system_theme_data() a second time. Nothing should break!

And here's the problem: The information from _system_theme_data() contains objects, which are passed as pointers (by-reference, if you want). Now the information from system_get_files_database() is stored in these objects and thus cached in the static variable of _system_theme_data(). A second call to system_get_files_database() will not overwrite these values.

----

Please, guys, have a second look at this!
The other issue contains a patch.

----

EDIT:
Reading more of the old posts, I noticed that #27 (Dave Reid) has already seen this. And #39 and #42 already propose to clone stuff. So, I was too fast, again.

But:
For some reason the clone idea was rejected for the final patch. The issue focused on fixing update.php. But, #632080 shows there are other situations where the side effect is undesired. The one situation that I found might be a rare case (only applies with devel automatic redirection turned on), but I would not be surprised for other situations where system_theme_data() is called twice.

I think the clone idea should be reconsidered.
If this all worked in PHP4, which does automatically clone everything, I don't see why explicit clone would cause any problems. The safest would be to always clone, the only problem with this might be performance.

subscribing...

Does this explain why i get this error message ?

warning: array_map() [function.array-map]: Argument #2 should be an array in /Applications/MAMP/htdocs/elephant_site_pilotage_dev_v_0_1/modules/system/system.module on line 1019.
warning: array_keys() [function.array-keys]: The first argument should be an array in /Applications/MAMP/htdocs/elephant_site_pilotage_dev_v_0_1/includes/theme.inc on line 1817.
warning: Invalid argument supplied for foreach() in /Applications/MAMP/htdocs/elephant_site_pilotage_dev_v_0_1/includes/theme.inc on line 1817.

I recently switched from one theme to another one, so this could come from elsewhere I just wonder.

Subscribing

@donquixote is correct - this bug is not fixed yet.

All it takes to trigger this again is one contrib module which happens to call system_rebuild_theme_data() somewhere in the update process. I experienced some pain with this last week, and I think the "culprit" wound up being in Drush, although it doesn't really matter since this is ultimately a core bug... In fact, this actually occurs in core alone right now, but only during the D6->D7 upgrade, since block_update_7004() forces a theme rebuild. Sites upgrading from D6 to D7 currently get all their themes disabled.

We need to make system_rebuild_theme_data() safe to call from anywhere. I think the simplest fix is just to remove the static cache from _system_rebuild_theme_data(). Drupal has a habit of statically caching everything just because it can, but this is a bad habit :) In particular, this function is designed to only get called very very rarely, in cases where we don't care much about optimizing performance. Furthermore, to actually take advantage of the static cache requires that it be called twice on the same page load. That may well be happening, but I would suggest that if system_rebuild_theme_data() is actually getting called twice on the same page load, that is something that should be fixed in the calling code.

Removing the static cache also has the following benefits:

1. We get rid of some random drupal_static_reset() calls that are pretty ugly.
2. We have two places in core that currently call _system_rebuild_theme_data() without ever using its return value, which can serve no purpose unless that somehow bizarrely primes the static cache for real use later on? The one in modules/system/system.updater.inc (see attached patch) seems like it's surely a bug that prevents that functionality from working correctly (I haven't tested, though).
3. This static cache caused some bizarre problems in #651086: Cache clearing is an ineffective mess in module_enable() and system_modules_submit() as well, and I found at the time that removing it helped things a lot.
4. Finally, I think the original fix committed above introduced a bug of its own? When running update.php, we do need to make sure that the rebuilt theme data gets inserted into the database as early as possible (because people very commonly go to update.php after changing the code in their filesystem, so the data in the {system} table is likely out of date and needs to be replaced). I have added a code comment to this effect in the attached patch - one could reasonably argue that update_check_incompatibility() is not the best function to do this in, but changing that would be outside the scope of this issue.

Note that the attached patch is much smaller than it looks, since most of it is just indentation changes after removing the static cache. My light testing suggests that this does not noticeably affect performance during installation or updates, but if it does, as mentioned above I think we need to look at why there is calling code that rebuilds the theme data multiple times in a row, and fix it there - probably in a separate issue.

I think we should find all cases when we really need to scan themes' files.

PS: http://api.drupal.org/api/function/_system_rebuild_theme_data/7 does not show all references to this function...

Yes, several places in core probably still scan the filesystem and do a full rebuild, even though calling http://api.drupal.org/api/function/system_get_info/7 or an equivalent custom database query would be sufficient. I think that is probably a separate issue.

For _system_rebuild_theme_data(), I think the only place this patch misses that we might want to change someday is list_themes(), which does this:

    if (!defined('MAINTENANCE_MODE') && db_is_active()) {
      foreach (system_list('theme') as $theme) {
        if (file_exists($theme->filename)) {
          $theme->info = unserialize($theme->info);
          $themes[] = $theme;
        }
      }
    }
    else {
      // Scan the installation when the database should not be read.
      $themes = _system_rebuild_theme_data();
    }

In theory we want to remove the MAINTENANCE_MODE check and leave only the db_is_active() one. Naively removing it leads to some subtle errors, though, so I did not include it in the above patch... I think it would be better as a potential followup issue.

I'm not sure if that static is new or old, but system_rebuild_theme_data() in system_install() is likely one of the reasons for #686196: Meta issue: Install failures on various hosting environments. This whole area is a huge mess, but if we remove the static we also need to ensure it's not being called twice in the same request in core anywhere, otherwise our memory / timeout issues are only going to go downhill.

MAINTENANCE_MODE check discussion #719850: errors in update.php during bootstrap yield a blank page

I just want to point out that D7 and D6 look very different. system_rebuild_theme_data() does not even exist in D6. We will need different solutions for each.

Shouldn't we get #632080: _system_theme_data() should clone the returned objects. into D6 already? There will be no classical backport path, if D7 looks so different.

@donquixote, system_rebuild_theme_data() was previously called system_theme_data() in D6, but otherwise I don't think they're that different. However, in D6 we definitely want the least disruptive fix possible.

@catch, it looks to me like the static was added in Drupal 6. (Interestingly, it doesn't seem like there was ever an equivalent static for scanning modules, even though I assume that is more expensive since most sites have a lot more modules than themes lying around...)

I checked for duplication and it does appear that on every page request during install or update, the maintenance theme is needlessly calling _system_rebuild_theme_data() twice. The attached patch adds a fix for that by simply reusing list_themes(), which has its own static cache. This gets it down to a maximum of one call in most cases. However, we still get multiple calls when we are explicitly rebuilding the theme data (one call for the theme system, one call for the rebuild) - and system_install() is an example of that. However, I don't get the impression that system_rebuild_theme_data() is the bottleneck for the crazy timeout issues in #686196: Meta issue: Install failures on various hosting environments at all - do you? That function call was there in D6 as well, and doesn't seem to have changed much internally except for the stream wrapper stuff. Probably the solution to that issue is going to involve breaking system_install() up into multiple steps anyway.

Also, I would assume that getting rid of a needless static cache would (if anything) reduce memory consumption rather than add to it, right? Some quick testing with the command line installer suggests that this is the case, although the reduction is probably negligible, ~35 KB or so.

Re: David Rothstein's patch (#103, #109)
Totally removing a static cache from system_theme_data() / system_rebuild_theme_data() will invite calling functions (core or contrib) to introduce their own static cache for the same data. These new static caches could be even more out of control than the existing one.

So, wouldn't it be better to think about what exactly we want to cache, and when it needs to be reset?

-----------

system_rebuild_theme_data() was previously called system_theme_data() in D6, but otherwise I don't think they're that different. However, in D6 we definitely want the least disruptive fix possible.

Interesting!
Some thoughts (a bit unstructured and brainstormy-like):

I see different cases:

1. We scan the filesystem for theme definitions, and write that information to the database. This is what "rebuild" is about. Scanning the filesystem should not be done more than 1x per request - trying to catch up with possible file changes during one request would be insane. So, we can safely cache this information in a static var, BUT we should really only cache the stuff that is coming from the filesystem, not anything else!
2. Usually a rebuild process takes more than just information from the filesystem. In our case, it depends on previous contents of the system table (theme enabled or disabled, etc). If we cache this info, we need to make sure it is reset when necessary.
3. We load stuff from the database (system table), but we don't rebuild anything. We can cache this, but we need a reset option. The reset will only be called if some other function is writing to the database (switching a theme on or off, etc), or doing a full rebuild. It can be a lazy reset, and we don't need to re-scan the filesystem.

It would be nice if these differences were reflected by code / function names / documentation with more clarity.

The new function name suggests that calling the function will actually do a rebuild. Thus, with the new name it is only logical to remove the cache. Still, there could be some functions that don't need a rebuild, but just want the data from the database, or from a static cache. Should we provide a separate function for this use case? Or does it exist already?

----------

Some useful links:
http://api.drupal.org/api/function/system_theme_data/6
http://api.drupal.org/api/function/_system_theme_data/6
http://api.drupal.org/api/function/system_rebuild_theme_data/7
http://api.drupal.org/api/function/_system_rebuild_theme_data/7

Btw, I noticed that _system_rebuild_theme_data() still has the same problem of objects that are not cloned.

I forgot to mention above that I also tried poking around other pages in Drupal that would be expected to call system_rebuild_theme_data() and found that most only call it once per page request currently (visiting the modules page, submitting the modules page, hitting the "clear caches" button, etc). Visiting admin/appearance was an exception. There it is called twice, once by the menu callback function and once by update.module when adding its hook_help().

Other than that, it appears that outside of install.php and update.php, this static cache has zero benefit in Drupal core itself.

Still, there could be some functions that don't need a rebuild, but just want the data from the database, or from a static cache. Should we provide a separate function for this use case? Or does it exist already?

There is http://api.drupal.org/api/function/system_get_info/7 which is useful for some of these cases, but not all of them.

One thing that does occur to me, if we want to retain some kind of static cache - perhaps what we really want to retain is the fact that the filesystem was already scanned on this page request? But we don't necessarily need to cache all the data itself. It should be cheap to get that from the database again. That would not help install.php, but would be very clean for everywhere else. Something like:

function system_rebuild_theme_data() {
  $file_scan_performed = &drupal_static(__FUNCTION__, FALSE);
  if (!$file_scan_performed) {
    // ... Do exactly what the function does now. Then before returning, set this...
    $file_scan_performed = TRUE;
  }
  else {
    // Query the database and return everything in the same format that is returned above.
  }
}

This would have the advantage that multiple modules could declare "I really need to make sure I'm getting the most up-to-date data from the filesystem" without ever triggering multiple filesystem scans on the same page request (and without maintaining a giant buggy static cache that is rarely going to be used).

I think that any of these more complex solutions are good, but require more of a rewrite of these rebuild functions than we ideally want to do in this issue :) On the other hand, it's quite possible that some kind of major rewrite is going to be required to correctly solve #651086: Cache clearing is an ineffective mess in module_enable() and system_modules_submit() anyway.

Did a bit of digging.
_system_rebuild_theme_data() gets everything from the filesystem, scanning .info files. The scan depends on the site name, but we can assume that this won't change during one request.

There is one exception: drupal_alter('system_info', ..). Duh! It can easily happen that this will yield different results if called a second time, if in the meantime we changed some configuration here and there.
http://google.com/codesearch?hl=de&lr=&q=function%5C+%5Cw%2B_system_info...

Aside of this one issue, it would be safe to cache the result of _system_rebuild_theme_data() in a static var, and this is exactly what happens in the current implementation. The idea is to cache what is coming straight from the filesystem.

But, there is another problem, more severe than the drupal_alter() thing: The result of the cache is returned as an object, (that means, by reference / as a pointer, or how you prefer to call it). This means that calling code can easily change the contents of the static cache, and it does! Which totally kills the idea of caching only what comes from the filesystem. Which is exactly what #632080: _system_theme_data() should clone the returned objects. attempts to fix. As I see now, #632080 can be perfectly adapted for D7.

With #632080 done, the remaining issue will be the drupal_alter() call.

Solution: Split the function in two parts.
The first does only scan the *.info files, and cache that information in a static var. The second will take this information, call drupal_alter(), and do some minor computations to determine the base theme etc (this happens after drupal_alter(), so we can't cache it). This second function can also have a cache, but if it does, we need to make sure that we don't forget to reset it when necessary.
Both parts of the function will clone the cached data, and never return the original, OR the data has to be wrapped into an object with read-only interface.

We could also implement this cache directly into file_scan_directory(), so it would also work for *.module or *.tpl.php scans.

I could prepare a patch for this solution, but I think we already win much with #632080 alone.

-----------

One thing that does occur to me, if we want to retain some kind of static cache - perhaps what we really want to retain is the fact that the filesystem was already scanned on this page request? But we don't necessarily need to cache all the data itself. It should be cheap to get that from the database again.

I could agree with this, but:

• The information from file_scan_directory() is modified before it goes to the database, and the modifications can depend on a lot of dynamic things, unpredictable for us. The contents of the system table does not reliably reflect the contents of the info files.
• _system_rebuild_theme_data() does not directly write to the system table. In fact, we have no guarantee that a function (indirectly) calling _system_rebuild_theme_data() will ever write anything to the database. This means, the second time that _system_rebuild_theme_data() is called, the database could still contain outdated content. We don't want to change this, because _system_rebuild_theme_data does not have all the information it would need, before it can write to the system table.

If you want to replace the static cache with a db solution, you need to introduce a place in the database to store the raw, unaltered *.info data. This could be a good idea anyway, to avoid too many directory scans! Still, I think a static cache wouldn't hurt that much:

maintaining a giant buggy static cache that is rarely going to be used

One side effect of the "clone the returned objects" is that the original cache will not consume as much memory as it would if other modules have the chance to stuff it with additional data. For a site with 20 themes, this is the data from 20 theme info files - usually not that much, compared to other data we keep in memory. And the few people that have 50 or more themes on one site can be expected to have bigger memory.

Very simple D7 clone patch.

late night brings stupid mistakes

..

Duh!
drupal_system_listing() depends on the install profile:
global $profile
I hope this doesn't change too much.

A more complete solution that attempts to solve the drupal_alter problem as described in #112.
Probably buggy. I don't have a D7 testing infrastructure at the moment.

I think we should do #115 first, and then plan this as a second step.

Btw, _system_rebuild_module_data() has the same problem.
EDIT: No, it does not. module data is not cached.

the usual problems: line endings etc.

#118 is going to break thanks to a missing ")".

Alternative solution, where the cache cloning goes into a reusable function.
Probably buggy.
I still recommend #115 for an immediate solution.

Finally have my testing environment :)

Same as #123, but replaced $themes[$key] with $theme in the foreach() loops.
Btw, I have the feeling that system_find_base_themes() could be a lot simpler.

EDIT: "Image field validation tests (ImageFieldValidateTestCase) [Image]"
How is this related??

#123 had a print_r().
And sorry for the patch spam. If you know a better way, let me know.

I fail to understand why #124 fails, while #126 is fine.

Plan of action:
We can either go for #115, which doesn't need much change imo, and postpone #126.
#126 was written with patch readability in mind, it needs some cleanup before commit.
The changes in #124 are purely cosmetic.
I wait for feedback from others, before I continue.

@David Rothstein:
#109 might be an alternative, but ...
could you upload a version of #109 without the indentation changes? if (TRUE) can do wonders :)
Patch reviewing on Windows is a pain.
(or is this too much lazyness on my side?)

Let's go with one of the simpler solutions to fix this bug and postpone the rest until later. I think the other work you started might definitely come in handy over at #651086: Cache clearing is an ineffective mess in module_enable() and system_modules_submit() though...

Actually, whichever patch we go with here should contain most of the cleanups I had in #109, since those were generally in the codebase as a workaround for this bug and therefore aren't (or at least shouldn't be) necessary anymore. Removing them as part of this patch is therefore important and helps test that it works. I've also gone ahead and written an actual test as well :)

So, attached we have:

• A patch which removes the static cache, based on #109, and removes all the workarounds.
• A patch which clones the static cache, based on #115. It turns out we still can't get rid of the drupal_static_reset() calls in this case, because it leads to the infamous "toolbar in the overlay" bug from #651086: Cache clearing is an ineffective mess in module_enable() and system_modules_submit() - so with this solution, the static cache is still somewhat problematic and requires a workaround or two - at least until we deal with that other issue. (However, the other workarounds are removed in this patch.)
• A patch which only removes the workarounds and adds the test. This is just to make sure the test fails when we want it to.
• Another version of the first patch, but without all the indentation changes. This is just for reading and ease-of-reviewing pleasure :)

Let's let reviewers decide which to go with.

Either of the first two seem OK to me, but I personally prefer the first one a bit since I think it leads to a simpler, cleaner codebase. Plus, as mentioned, we already don't have a static cache on _system_rebuild_module_data(), even though that function is probably more expensive than this one (and gets triggered two or even three times per page request when visiting admin/modules) - and the world hasn't ended as a result of that :)

Well, that proved my point - but now let's reupload the actual two patches so the testbot will leave things at "needs review" :)

While the clone cache patch looks OK, the remove cache patch has dodgy comments in it:

+  // Find themes
+  $themes = drupal_system_listing('/\.info$/', 'themes');
+  // Find theme engines
+  $engines = drupal_system_listing('/\.engine$/', 'themes/engines');
+

I think David's right that the themes directory is usually much smaller than the modules directory, so could probably live without the static cache otherwise.

All I did in the middle part of that function was adjust the indentation due to the removal of the if() statement. I didn't try to improve any of the existing dodgy comments :)

andypost pointed out in #632080-11: _system_theme_data() should clone the returned objects. that there are some D6 sites such as ThemeGarden, that use a lot of themes.

If we notice performance issues after removing the cache, I propose that we add a cache to drupal_system_listing(). This can be discussed in a separate issue.
With a clever implementation, this cache will be totally independent of global state, and will only have to be reset if someone actually does change in the filesystem.

Would we need to be afraid of memory issues? I think not. We will only cache a minimum of data for every file found in a filesystem scan, and we make sure that no other functions can spoil this cache with additional data. Other cached variables are bigger - think of theme registry.

First patch from #130 looks RTBC. Just in case, it's this one: http://drupal.org/files/issues/system-rebuild-theme-data-305653-128-REMO...

This patch no longer applies. Can we get a quick reroll of the 'REMOVE-CACHE' patch?

Hopefully I got everything...

#136 is plain wrong lots of errors

One day someone will add a period to that comment.

Committed to CVS HEAD. Thanks.

Simple followup patch allowed by this: #758804: Do not unnecessarily rebuild module and theme data in the installer

Nice, nice.
So what about D6 ? Remove cache, or rather clone?

I think D6 just needs clone, because removing could break some custom code.

Okay, I'm just talking out of my hiney here, but while David Rothstein says in #103 "All it takes to trigger this again is one contrib module which happens to call system_rebuild_theme_data() somewhere in the update process." -- well, it seems to me this problem transcends the update process (or cron, as documented separately).

I stumbled upon this issue when I began receiving these errors in D6.15:

* warning: array_map() [function.array-map]: Argument #2 should be an array in /path/to/modules/system/system.module on line 1015.
* warning: array_keys() [function.array-keys]: The first argument should be an array in /path/to/includes/theme.inc on line 1771.
* warning: Invalid argument supplied for foreach() in /path/to/includes/theme.inc on line 1771.

and experienced my default theme being disabled. It's still the default, but it's no longer enabled -- and the system table no longer has an entry for that -- and only that -- theme.

However...

This is happening neither during an update nor a cron run (at least, not when I run cron manually) -- it's happening at least daily. I am at my wits end with this problem. Am I wrong to think my problem is related to this issue?

(BTW, a Google search of this error brings up hundreds of sites that are experiencing this error.)

same here. I re-enable the themes, do what i need to do, which is usually moving blocks around. I've installed Organic Groups, too. It seems like the themes will work, but I need to re-enable them any time I need to make adjustments that call on the themes, like blocks does. It's making me very nervous. I wish I knew more about how to help, testing or anything.

It seemed to me that the problem was occurring after a status update and -- sure enough -- a check of the core Update module shows that system_theme_data() is called there, too. My problem was specifically caused by me playing around with multiple site configurations, using the same database, and a cron job for two sites running at the same time. My fault, but it does point out that there's another point of failure for the system_theme_data() function.

For clone there's patch already #632080: _system_theme_data() should clone the returned objects.

sub

subscribing...

#128: system-rebuild-theme-data-305653-128-TESTS-SHOULD-FAIL.patch queued for re-testing.

@manos_ws dont try to re-test old patches. The question is asked in #144

This is indeed frustrating many administrators of drupal sites. Will there be a solution in 6.17?

subscribe

Another cause of this error would be changing the permissions of sites/default/themes so that the default enabled theme were no longer readable.

-B

subscribing

does just using one theme work around the problem?

perhaps this is related: http://drupal.org/node/794736 ?

Happens also with just one (fusion) theme activated. Does not only occur while updates, but apparently after cron. Also after applying the clone patch.

edit: same case as mrtoner describes in #146. but without multisite configurations...

does not occur with the default garland theme, but with fusion (core) and the skinr module.

Closed for me.
I guess this was caused by some other module, as it is magically fixed now - no patching to core (using vanilla 6.16). Now the problematic site can updatedb with no problems (and still retain its 10+ themes active).

not for me.

the only way to work around this problem was selecting the garland theme. even if i had just the acquia marina or zen theme active it suddenly got deactivated. really ugly bug.

I'm just happy my iteration of this bug got magically fixed :)

There's a patch for D6 #632080-17: _system_theme_data() should clone the returned objects.
Suppose this is enough to fix

@andypost, the patch did not fix it in my case.

subscribe

After the D7 commit (comment #142), is this bug affecting just minor D6->D6 updates now, or still affecting major D6->D7 upgrades (like in #103)? Is the D7 upgrade path tag (blocking Drupal 7 beta) still valid here?

Anyway, during upgrades between major versions (D6->D7) it's recommended to switch to a core theme, such as Garland...

subscribing

Is this supposed to be fixed in D6.19? I am asking because I just got hit by the disabled themes bug while testing Open Atrium 1.0b8 (that is based in the D6.19 code).

I tried reading through the whole issue -especially after what's been done during 2010- and I am quite lost from successive switching back and forth between 6.x and 7.x and various commits. So, what do I need to test in order to help with this issue? Should I apply #17 from #632080: _system_theme_data() should clone the returned objects. and see?

Same question as above, what's going on here?

ehrm, it seems like the component 'update system' no longer exists, and that was why the component was changed.

Same problem here on D6.20: after updating a contrib module, all the enabled themes were disabled.
I'm not sure on what to to now... maybe apply #632080: _system_theme_data() should clone the returned objects. ?

Subscribing.

From #144:

So what about D6? Remove cache, or rather clone?

Well, I'll join the queue here wondering what to do with a clean/simple D7.4 install that exhibits this problem. It's one of two sites managed through aegir...the other one (Bartik based) keeps its theme set while this one (using Marinelli) does not. Watchdog shows:

Warning: uasort() [function.uasort]: The argument should be an array in system_themes_page() (line 237 of /var/aegir/platforms/d7smallsite-222/modules/system/system.admin.inc).

I'm also taking the (perhaps ill-advised) step of setting this to 7.4 in hopes of at least having the proper issue thread shouted at me...

I think you have another issue there, I'd recommend opening a complete new issue with that error message, and ideally steps to reproduce from a clean install. Moving this back to 6.x.

Okay, thanks catch for the advice: I just did not want to fork the issue unnecessarily.

subscribing

subscribe

@Fogg and @deepeddy: Please stop suscribing like this. Instead press the Follow button above the Issue Summary.

Is anything still happening in this issue? I am currently setting up a new drupal page and have this issue. Looks like the themes are deactivated over night - every night.

I am running a multisite installation and on the same installation at least 3 additional drupal pages are installed and working just fine. So I would guess the issue is with some module/theme. Any ideas where to look?

Thanks,
Fogg

I have not seen this issue occur since the patch from
#632080: _system_theme_data() should clone the returned objects. is in core.

However, there could be other reasons for this still happening to other people.
You need to investigate yourself, using watchdog() and friends.

Hi everyone. My themes are disabling constantly.

I am running drupal 6.22 and everything is up to date. I am using Themekey too.

This bug does not occur on the identical site on my local only on the live server!!!

Can the above patch be used on this update or is it already in core now and I need something else.
I have tried so many things with no luck.

Does anyone know of a workaround. Could the database tables be locked in some way so the themes cannot be disabled????

Any other clever fix suggestions?

This bug continues to haunt me at frequently but inconsistently. It has been going on for more than two years and I am getting tired of always going to admin/build/themes.

From what I can tell, my problem is not tied to either cron or update.php. Rather it occurs as a result of serving some unknown page that calls one of about 50 modules installed on the website. My suspicion is that its in fact one or more malformed contrib/custom modules that is causing the root problem. I cannot provide more information yet, but am starting to do further debugging because it simply is so frustrating to deal with.

Solving this will be sort of a Christmas present to myself.

Maybe you could modify your _db_query() or db_query(), and catch any
"UPDATE {system}" or "DELETE FROM {system}".
And then a SELECT to check if any theme is missing or disabled, make a backtrace and log it (watchdog or something else).

Yep, there is already a mechanism for this kind of thing - just look at _db_query() in database.mysqli.inc.
But, a custom hack could be a lot cheaper on system resources.

Check how devel query log does it.. and then do it whatever way you want :)

Here is some code that could help you with debugging.
No guarantees, just as a starting point.

It is a replacement of db_query() in database.mysql-common.inc, for D6.

function db_query($query) {
  static $_stack_count = 0;
  $babysit_themes = !$_stack_count && preg_match('/^UPDATE {system}/', $query);
  ++$_stack_count;
  $args = func_get_args();
  array_shift($args);
  $query = db_prefix_tables($query);
  if (isset($args[0]) and is_array($args[0])) { // 'All arguments in one array' syntax
    $args = $args[0];
  }
  _db_query_callback($args, TRUE);
  $query = preg_replace_callback(DB_QUERY_REGEXP, '_db_query_callback', $query);

  if ($babysit_themes) {
    $sql = "SELECT name, filename, status FROM {system} WHERE type = 'theme'";
    $rows = array();
    $q = db_query($sql);
    while ($row = db_fetch_object($q)) {
      $rows[$row->name]->before = $row->status;
    }
    $result = _db_query($query);
    $q = db_query($sql);
    while ($row = db_fetch_object($q)) {
      $rows[$row->name]->after = $row->status;
    }
    foreach ($rows as $name => $ba) {
      if ($ba->before && !$ba->after) {
        // A theme was disabled. This might be intended, or not.
        $msg = '';
        foreach (debug_backtrace() as $call) {
          $msg .= "\n<tr><td>$call[function]()</td><td>in line $call[line] of '$call[file]'</td></tr>";
        }
        $msg = '<table><tbody>'. $msg .'</tbody></table>';
        _babysit_theme($name, $msg);
      }
      elseif ($ba->after) {
        _babysit_theme($name, FALSE);
      }
    }
  }
  else {
    $result = _db_query($query);
  }

  --$_stack_count;
  return $result;
}

function _babysit_theme($name = NULL, $msg = NULL) {
  static $_memory;
  if (empty($name)) {
    // We are on shutdown.
    foreach ($_memory as $m_name => $m_msg) {
      watchdog('babysit theme', "<p>Theme '$m_name' was disabled.</p>$m_msg");
    }
  }
  else {
    // Called from db_query().
    if (!isset($_memory)) {
      // That's the first run.
      $_memory = array();
      register_shutdown_function(__FUNCTION__);
    }
    if ($msg === FALSE) {
      // False alarm, this theme is enabled again.
      unset($_memory[$name]);
    }
    else {
      $_memory[$name] = $msg;
    }
  }
}

What it does:
Whenever a theme is disabled, and is not re-enabled during the same request, then it will write the stack trace of db_query() to the watchdog.

Thank you for the suggested debugging function @donquixote. I am not at the right computer right now, but late this afternoon, I will incorporate your function into the code on the troubled site. Hopefully, this and some of my own debugging statements will help us pin down better where this elusive bug might be coming from. FYI - The troubled site is running Drupal 6.20.

@donquixote - Last night I stalled the code in #186 as you suggested. Checking today, I see that I have had several instances of the themes becoming unset. However, there is no output/watchdog data from the above code. I will need to debug this new code to make sure I completely understand what it is doing and where it might not be working as designed.

The log files do show that one occurrence must be somehow related to the hook_cron() code in the update module. The other random occurrences do no yet make any sense.

Maybe the preg_match('/^UPDATE {system}/', $query) is too strict?
Maybe you also need to catch "DELETE FROM {system}".
And be more tolerant with whitespace and linebreaks.

Hi all,

I am facing the same issue on of our my sites in a multisite environment. May be as additional input:

I cannot believe that a permission on the cronjob should be the issue. What I have found today is
a) baseurl was setup wrong (www2.domain.com instead www.domain.com)
b) cron.php was called multiple times from different crontabs (at the same schedule)

I changed this now, will update this issue within the next days if it works now or not.

Fogg

my issue is solved. Most likely the wrong baseurl was the root cause. Hope this helps also others.

I also had this happen to me again.
Looks like we need a way to catch what is going on.

Suffered from this problem a few times in 2012 on Drupal 6 sites. Especially when updating to the latest Drupal 6 core.

Do not know if my problem have the same reason
but on run drush updatedb I got

Duplicate entry 'themes/garland/minnelli/minnelli.info' for key 'PRIMARY'                                                     [warning]
query: INSERT INTO system (name, owner, info, type, filename, status, throttle, bootstrap) VALUES ('minnelli',
'themes/engines/phptemplate/phptemplate.engine',
'a:14:{s:4:\"name\";s:8:\"Minnelli\";s:11:\"description\";s:56:\"Tableless, recolorable, multi-column,
fixed width

the the same for all my themes.
Looks like the problem was in function system_theme_data() in this logic case

if (isset($theme->status) && !(defined('MAINTENANCE_MODE') && MAINTENANCE_MODE != 'install')) {
        db_query("UPDATE {system} SET owner = '%s', info = '%s', filename = '%s' WHERE name = '%s' AND type = '%s'", $theme->owner, serialize($theme->info), $theme->filename, $theme->name, 'theme');
      }
      else {
        $theme->status = ($theme->name == variable_get('theme_default', 'garland'));
        // This is a new theme.
        db_query("INSERT INTO {system} (name, owner, info, type, filename, status, throttle, bootstrap) VALUES ('%s', '%s', '%s', '%s', '%s', %d, %d, %d)", $theme->name, $theme->owner, serialize($theme->info), 'theme', $theme->filename, $theme->status, 0, 0);
      }

here is a patch for this

@igor.ro: see #1425868: Duplicate entry of themes primary key in systems table of Drupal 6.24 (using Drush or Ægir).

warning: array_map() [function.array-map]: Argument #2 should be an array in /var/www/clientsdomain/httpdocs/modules/system/system.module on line 975.
warning: array_keys() [function.array-keys]: The first argument should be an array in/var/www/clientsdomain/httpdocs/includes/theme.inc on line 1728.
warning: Invalid argument supplied for foreach() in /var/www/clientsdomain/httpdocs/includes/theme.inc on line 1728.

I updated using drush and after that accessing the page in the browser it had thrown the above. I realized the trouble starts
if you use for example a framework for the theme. In my case that is blueprint. The same applies for fusion framework.
If your theme is in a path like /sites/all/themes/blueprint/mytheme or /sites/all/themes/fusion/mytheme then drush deletes the subdirectory of a theme.

So, if you use subthemes make sure you put the subtheme on the main themes/ path before you run update.
I read somewhere it is no longer important to have a subtheme in the subdirectory of the main theme/framework, in my case this solves the problem.

how to add new image to website. please update panelistas website

I am removing the tags used in just few issues. I apologize for bumping this issue.