Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Status: Needs review » Needs work

Patch failed to apply. More information can be found at http://testing.drupal.org/node/14307. If you need help with creating patches please look at http://drupal.org/patch/create

pillarsdotnet’s picture

Status: Needs work » Needs review
FileSize
17.75 KB

Okay...

Hopefully testbot will like this version?

Pasqualle’s picture

Version: 6.4 » 6.x-dev

I am not sure that this could get in, because only bug fixes are accepted for D6..

pillarsdotnet’s picture

Category: feature » bug

Re: Pasqualle "only bug fixes are accepted"

There are race conditions caused by two or more webserver processes calling the named functions.

The race condition caused errors in my installation; my bug report got marked "wontfix".

This patch should greatly reduce (but not eliminate) the chance of such a race condition.

it is my understanding that if the D7 patch lands then the D6 backport has a good chance of being accepted also.

Should a backport of a performance enhancement which reduces but does not completely eliminate a known bug condition be marked as a feature request or a bug report?

pillarsdotnet’s picture

Checked out cvs branch DRUPAL-6 (currently 6.5-dev), applied the patch, and tested. Found several errors and fixed them.

Corrected patch created via "cvs diff -u -p" attached

pillarsdotnet’s picture

More errors fixed; re-rolled. Seems to be working well, now.

pillarsdotnet’s picture

FileSize
17.85 KB

Re-rolled against current CVS checkout. To apply:

cvs -z6 -d:pserver:anonymous:anonymous@cvs.drupal.org:/cvs/drupal co -r DRUPAL-6 drupal
wget -O - http://drupal.org/files/issues/147000_backport-to-6.x.diff | patch -p0 -d drupal
keith.smith’s picture

Status: Needs review » Needs work

On a code style review, many comment lines should end with a period.

Also,

+      // Keep the old filename from the database incase the file has moved

"incase" -> "in case"

pillarsdotnet’s picture

Status: Needs work » Needs review

Oh, give me a break! You're marking this "code needs work" based on the fact that I didn't correct grammatical errors in the comments???

Go bother the original developers; I'm just maintaining a backport.

Dave Reid’s picture

Comments that I added have been revised in 147000#comment-1050505. Comments already in core, that's a quite different story...

pillarsdotnet’s picture

Incorporated Dave Reid's six lines of comment correction.

Re-rolled patch; apply as before.

pillarsdotnet’s picture

Re-rolled patch aganst current CVS checkout; should also apply against 6.8.

dmitrig01’s picture

Couple of no-no's here:

+	db_query('UPDATE {system} SET ' . implode(',',$sql_sets) . ' WHERE filename=\'%s\'',$sql_args);
+      }
 
-    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, isset($theme->status) ? $theme->status : 0, 0, 0);
+      // Remove the file so that we don't try to insert it later.
+      unset($files[$file->name]);
+    }
+    else {
+      // File is not found in filesystem, so delete record from the system table.
+      db_query('DELETE FROM {system} WHERE filename=\'' . $file->filename .'\'');

please re-write this to be

+	db_query("UPDATE {system} SET ". implode(', ', $sql_sets) ." WHERE filename='%s'",$sql_args);
+      }
 
-    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, isset($theme->status) ? $theme->status : 0, 0, 0);
+      // Remove the file so that we don't try to insert it later.
+      unset($files[$file->name]);
+    }
+    else {
+      // File is not found in filesystem, so delete record from the system table.
+      db_query("DELETE FROM {system} WHERE filename = '%s'", $file->filename);

also please replace all tabs with 2 spaces.

Damien Tournoud’s picture

Status: Needs review » Closed (duplicate)

Sorry, but this is a duplicate of #147000. Don't open duplicate issues for different versions. Let's get#147000 in, and then think whether we should backport it or not.

pillarsdotnet’s picture

Assigned: Unassigned » pillarsdotnet
Category: bug » task
Status: Closed (duplicate) » Needs review

Anybody running 7.x has bigger problems than the ones cured by this patch. If-and-when #147000 gets integrated, any 6.x users who ask for a backport will no doubt be told to upgrade to 8.x instead. I'm okay with this, but it doesn't mean that I'm going to stop running, fixing, and sharing code.

I'm sorry if that offends you.

Damien Tournoud’s picture

Status: Needs review » Closed (duplicate)

You would use your time much better if you helped the original patch get in. We never open two issues for the same bug/improvement in two versions of Drupal. Please play with the team, don't try to go solo.

pillarsdotnet’s picture

Status: Closed (duplicate) » Needs review
FileSize
18.13 KB

Applied trivial changes to comments and formatting and re-rolled for today's checkout of DRUPAL-6 branch.

vthirteen’s picture

does this patch still need review? or it's been committed to more recent versions of drupal 6?

tim.plunkett’s picture

What's the status on this?

AlexisWilke’s picture

I'm getting those errors once in a while. Will the patch here remove those? It was from last year, so I guess that was dropped? Do you need people to check it?

user warning: query: INSERT INTO system (name, owner, info, type, filename, status, throttle, bootstrap) VALUES ('pushbutton', 'themes/engines/phptemplate/phptemplate.engine', 'a:13:{s:4:"name";s:10:"Pushbutton";s:11:"description";s:52:"Tabled, multi-column theme in blue and orange tones.";s:7:"version";s:4:"6.13";s:4:"core";s:3:"6.x";s:6:"engine";s:11:"phptemplate";s:7:"project";s:6:"drupal";s:9:"datestamp";s:10:"1246481719";s:7:"regions";a:5:{s:4:"left";s:12:"Left sidebar";s:5:"right";s:13:"Right sidebar";s:7:"content";s:7:"Content";s:6:"header";s:6:"Header";s:6:"footer";s:6:"Footer";}s:8:"features";a:10:{i:0;s:20:"comment_user_picture";i:1;s:7:"favicon";i:2;s:7:"mission";i:3;s:4:"logo";i:4;s:4:"name";i:5;s:17:"node_user_picture";i:6;s:6:"search";i:7;s:6:"slogan";i:8;s:13:"primary_links";i:9;s:15:"secondary_links";}s:11:"stylesheets";a:1:{s:3:"all";a:1:{s:9:"style.css";s:27:"themes/pushbutton/style.css";}}s:7:"scripts";a:1:{s:9:"script.js";s:27:"themes/pushbutton/script.js";}s:10:"screenshot";s:32:"themes/pushbutton/screenshot.png";s:3:"php";s:5:"4.3.5";}', 'theme', 'themes/pushbutton/pushbutton.info', 0, 0, 0) in .../drupal/modules/system/system.module on line 821.

Thank you.
Alexis Wilke

crea’s picture

subscribing

marcoBauli’s picture

subscribing and bumping. Tempted to set as critical, as is happening on several websites..

seanr’s picture

Priority: Normal » Critical

I'm getting these intermittently on nearly every one of my sites. A client just saw it today while my boss was training them - not good. #147000: Rewrite module_rebuild_cache() and system_theme_data() has already been fixed so this is no a valid backport.

bcobin’s picture

I'm getting this error also - what's the status here? Has the fix been backported to D6? I'm on 6.13 - what shall I do? Thanks much!

jbrauer’s picture

Assigned: pillarsdotnet » Unassigned
Status: Needs review » Needs work

Patch no longer applies. This leads to the loss of active themes on cron runs and there are a string of warnings about duplicate entries in the theme section of the system table.

jbrauer’s picture

Component: base system » system.module
Category: task » bug
Status: Needs work » Needs review
FileSize
15.58 KB

In my (very brief) testing this seems to solve the problem reported with cache rebuilding in #477550: "User warning: Duplicate entry ... " ... Themes and elsewhere. This is a re-roll of the patch in #17 to apply to 6.13.

bcobin’s picture

I get the following error message:

patching file install.php
can't find file to patch at input line 18
Perhaps you should have used the -p or --strip option?
The text leading up to this was:
--------------------------
|Index: includes/theme.maintenance.inc
|===================================================================
|--- includes/theme.maintenance.inc     (revision 96)
|+++ includes/theme.maintenance.inc     (working copy)
--------------------------
File to patch:

Any ideas? Thanks!

japerry’s picture

bcobin, to patch the file, you'll need to type 'patch -p0 < 307756-26-rebuild-cache-unification-backport.patch' inside the root drupal directory. For this example, the patch is also downloaded to the root drupal directory.

bcobin’s picture

Thank you, japerry - I was able to successfully apply the patch both on my development sandbox and on the production site. Woo hoo!

One note: On the production site, the patch changed permissions on update.php so it was not accessible, nor was I able to change permissions from FTP (I wasn't sure what the permissions should be anyway, though I'm sure I could have done it from command line.)

So I renamed update.php to update_old.php, copied update.php from the development install, and was able to run it, no prob.

I'll keep an eye out, but I am hopeful this squashes the nasty, nasty "theme error" bug! Thank you VERY much!

(The Drupal community ROCKS!)

seanr’s picture

Successfully applied to latest D6 release and so far it's looking good.

jbrauer’s picture

Status: Needs review » Reviewed & tested by the community

Several positive reports and no negative reports yet. Advancing to RTBC.

Dave Reid’s picture

Status: Reviewed & tested by the community » Needs work

We cannot change APIs in the Drupal 6 version. The patch would break any modules that use this data, and there are some modules that do.

AlexisWilke’s picture

Status: Needs work » Reviewed & tested by the community

Dave,

Which modules? What API is broken? It seems to me that all the old functions are kept. So far, I have not seen even on of the 100+ modules that I use having any kind of problem since I applied the patch!

So please, give us substance, who's the victim?! Without proof of otherwise it works!

Thank you.
Alexis Wilke

Dave Reid’s picture

Status: Reviewed & tested by the community » Needs work

Changing function names = changing API. The only times API can change are in extreme cases (absolutely necessary for security) and during major Drupal versions. For example, this patch changes module_rebuild_cache() to system_get_module_data(). Here's an example of just the DRUPAL-6--1 CVS modules that call that function:

grep -Rni 'module_rebuild_cache' .
./module_supports/module_supports.module:14:  $files = module_rebuild_cache();
./smartlist_manager/smartlist_manager.install:139:  module_rebuild_cache();
./patterns/components/system.inc:172:          $modules_info = module_rebuild_cache();
./patterns/components/system.inc:267:          $modules_info = module_rebuild_cache();
./patterns/patterns.module:845:  $modules_info = module_rebuild_cache();
./patterns/patterns.module:1351:  module_rebuild_cache();
./patterns/patterns.module:2600:  $modules_info = module_rebuild_cache();
./node_import/node_import.admin.inc:1246:  foreach (module_rebuild_cache() as $name => $module) {
./authcache/authcache.module:124:    module_rebuild_cache();
./simpletest_automator/simpletest_automator.module:405:  module_enable(array_keys(module_rebuild_cache()));
./xmlsitemap/xmlsitemap.install:414:  module_rebuild_cache();
./xmlsitemap/xmlsitemap.install:493:  module_rebuild_cache();
./mailman_manager/mailman_manager.install:116:  module_rebuild_cache();
./util/system_module.module:17:  $modules = module_rebuild_cache();
./testing_server_setup/testing_server_install/patches/core.patch:32:   $files = module_rebuild_cache();
./module_filter/module_filter.module:98:  $files = module_rebuild_cache();
./module_filter/module_filter.module:101:  foreach (module_rebuild_cache() as $id => $module) {
./configuration/configuration.module:2026:  module_rebuild_cache();    
./configuration/configuration.module:2060:  $modules_info = module_rebuild_cache();
./drush_mm/drush_mm.inc:114:  module_rebuild_cache();
./drupal_notifier/drupal_notifier.module:297:  $files = module_rebuild_cache();
./fivestar/fivestar.install:148:      module_rebuild_cache();
./fivestar/fivestar.install:152:      module_rebuild_cache();
./moduleinfo/moduleinfo.module:34:        $files = module_rebuild_cache();
./sitedoc/sitedoc.module:1144:  $files = module_rebuild_cache();
./luceneapi/luceneapi.admin.inc:30:  $files = module_rebuild_cache();
./module_browser/module_browser.module:68:    return module_rebuild_cache();
./features/features.admin.inc:12:  module_rebuild_cache();
./plugin_manager/plugin_manager.module:164:  $files = module_rebuild_cache();
./plugin_manager/plugin_manager.admin.inc:385:    module_rebuild_cache();
./plugin_manager/plugin_manager.admin.inc:424:  $files = module_rebuild_cache();
./services/services/system_service/system_service.inc:67:    $modules = module_rebuild_cache();
./provision/platform/install.php:243:  $files = module_rebuild_cache();
./provision/platform/provision_drupal.module:270:    $data['modules'] = _provision_drupal_get_cvs_versions(module_rebuild_cache());
./provision/platform/provision_drupal_clear.php:23:module_rebuild_cache();
./dtools/wsod/wsod.module:111:                module_rebuild_cache(); // rebuild paths in system table
./dtools/wsod/wsod.module:137:                    module_rebuild_cache();
./dtools/wsod/wsod.module:285:            module_rebuild_cache();
./dtools/wsod/wsod.module:294:            module_rebuild_cache();
./dtools/wsod/README.txt:31:    - run in emergency mode to execute module_rebuild_cache() to refresh module paths
./dtools/wsod/wsod_emergency.php:32:    module_rebuild_cache(); // so we need to rebuild path of modules
./webservices/wsservice_dcore/wsservice_dcore.system.inc:58:    $modules = module_rebuild_cache();
./simpletest/drupal_unit_tests.php:140:    module_rebuild_cache(); // Rebuild the module cache
./simpletest/drupal_test_case.php:290:      module_rebuild_cache();
./simpletest/drupal_test_case.php:316:      module_rebuild_cache();
./simpletest/drupal_test_case.php:453:      module_rebuild_cache();
./date/date_api.install:123:  module_rebuild_cache();
AlexisWilke’s picture

Status: Needs work » Needs review

Well... the module_rebuild_cache() function is still available. Now why he renamed them, maybe because in D7 it was renamed? And he used those from D7? But it is still D6 compatible because the old functions are still there. So, yes, we could spend time copying the function system_get_module_data() inside module_rebuild_cache() to make you happy, but I don't really see the point in wasting more time?! Another one that will break your code for real?

 function module_rebuild_cache() {
-  // Get current list of modules
-  $files = drupal_system_listing('\.module$', 'modules', 'name', 0);
+  return system_get_module_data();
+}
Chad_Dupuis’s picture

subscribing

seanr’s picture

Seems like #35 should satisfy the issue. Any reason we can't just add that to the patch and commit it?

jbrauer’s picture

Status: Needs review » Needs work

After consulting with folks this needs to be re-rolled using the existing functions where possible.

bcobin’s picture

Has any of this been implemented in 6.14? To wit; does 6.14 need this patch and, if so, will it apply correctly? Thanks...

jbrauer’s picture

The last patch above should apply to Drupal 6.14. However it will not be committed as is as it should be re-factored to use the existing functions instead of introducing new functions. For it to be included in Drupal 6.15 this needs to be done.

darkoba’s picture

I've tried to apply the patch on drupal 6.14 and the patch does not apply to the following files :

  • theme.maintenance.inc
  • common.inc
  • system.admin.inc
  • system.module

Could we have a patch which work with Drupal 6.14 please ?

SeanBannister’s picture

Just wanted to confirm I just received errors on my site that lead me to searching and finding this issue. Would really appreciate this being committed.

jbrauer’s picture

To clarify there is nothing here to commit. There is a patch that needs work. Updates to the patch are welcome so it can get reviewed and be considered for being committed.

The attached patch resolves the API issue and seems to resolve the issue with themes being disabled on cron. It does not, however, fix the issue of themes being disabled on update.php.

alison’s picture

Very sorry, but I've become confused after going through this thread, so I have a stupid question if you don't mind...

Is this latest patch, #43, intended to address (among other details) the "duplicate entry..." errors people are getting re: themes (enabled and disabled), as described in a number of places including here [#367966]?

If so, is the patch intended to be applied to Drupal 6.14 or the latest dev release (or...)?

(@jbrauer #43) Are you saying applying this patch will or may result in themes being disabled when update.php is run? (If so, can we simply re-enable the theme without additional problems? Or... is that part of what needs to be tested? or...?)

I'm completely on board with helping test this or other solutions, just want to make sure I understand what's up.

Thanks so much for your patience.

jbrauer’s picture

Is this latest patch, #43, intended to address (among other details) the "duplicate entry..." errors people are getting re: themes (enabled and disabled), as described in a number of places including here http://drupal.org/node/367966?

It has been effective for this.

If so, is the patch intended to be applied to Drupal 6.14 or the latest dev release (or...)?

It is built against 6.14 but should apply to head as well.

(@jbrauer #43) Are you saying applying this patch will or may result in themes being disabled when update.php is run? (If so, can we simply re-enable the theme without additional problems? Or... is that part of what needs to be tested? or...?)

This is correct. This is also why the patch is "needs work" because we cannot say the problem is 'fixed' but users after updating will have to go re-enable the themes. As a practical matter in the interim this will work but it needs to be a complete solution before it gets committed.

alison’s picture

@45 -- Thank you very much, your responses are perfect!

I will be trying the patch out later today most likely and will certainly comment again if anything unexpected happens -- and I'll be stopping by again when new comments are posted in case there's anything I can do to help with this. Thanks again, everyone!

ericm’s picture

subscribe. I'm getting what I expect are the same errors also noted in #477550: "User warning: Duplicate entry ... " ... Themes, after upgrading to 6.14.
user warning: Duplicate entry 'sites/all/themes/contrib_themes/amity_island/amity_island.info' for key 1 query: INSERT INTO system (name, ...

AlexisWilke’s picture

As a side note, because it is probably not directly related to this issue although the result was quite similar, the other day I moved a site from one server to another and somehow new themes were added (I use multi-site installs and there are more themes on the new server) and those addition did NOT include the proper type (i.e. the Type column was NULL!)

I simply ran an SQL order to fix the column, but your code would still fail because the DELETE expects the column to be proper (I would think it should be too!) and thus the INSERT would attempt to duplicate the path which is what the error is about, generally.

UPDATE system SET type = 'theme' WHERE type IS NULL;

IMPORTANT: I checked my table and could tell only themes had that problem, modules were properly defined.

Thank you.
Alexis Wilke

mikeytown2’s picture

BrightBold’s picture

Subscribing. I'm getting the Duplicate entry error as well.

micheleannj’s picture

subscribing

jbeall’s picture

Subscribing. It's been over a year for this "critical" bug (according to the current priority setting), I'm really looking forward to seeing this fixed!

-Josh

Sborsody’s picture

subscribing...

escoles’s picture

subscribing....when clients see this they get really hinky. will apply 6.14 patch today/tomorrow and try to test, but this happens routinely on the three sites I'm building at the moment. #45 was very helpful clarification.

opteronmx’s picture

I have the same issue, BUT I don't have cron task configured (as I'm still developing and I don't need cron to run automatically for now), and also never happened before when cron runs or update.php, it just happens. Was more notorious when content editing, but this also happens viewing content, or doing everything else, suddenly, all my themes are disabled and every time, all the site gets in a particular theme, the custom one, so at least visitors will always see custom theme while this bug gets fixed.

About time, doesn't happen in regular basis.

I checked #48 tip, but all my system table records are properly declared.

I discovered this bug days ago, but since thursday hadn't happen, but today, it happened again.

I'll try to apply patch when I find time to do so.

My setup:

Drupal 6.14 over WAMP server 2.0 (apache 2.2.8, php 5.2.6, mysql 5.0.5b) in dev server, but this happens too in future production server (LAMP I don't have versions at this time)

Modules installed:
ad, ad_flash, ajax, background, backup_migrate, better_formats, cck, cck_fieldgroup_tabs, chart, customized (Nothing fancy on this one), date, devel, email, emfield, filefield,
imageapi, imagecache, imagefield, jquery_ui, jquery_update, link, pathauto, smtp, spamspan, swftools, tabs, taxonomy_menu, token, transliteration, views, webform, xmlsitemap

Themes enabled:
Garland (for admin users) and custom theme (for visitor users)

escoles’s picture

I applied the patch at #45 to three sites, and the problem seemed to go away. However, just yesterday the client reported seeing it on one of the patched sites. I confirmed via the log that it had happened.

So the patch apparently doesn't solve the problem, just (possibly) reduces the frequency of occurrence.

ahankinson’s picture

Just had a problem with a Drupal multi-site install using a core with the patch from #43 applied. I was getting "duplicate theme" errors on a second site that I had set up. The problem was resolved by editing the database and manually inserting 'theme' into the 'type' column.

Looking at the patch, it removes the SQL statement:

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, isset($theme->status) ? $theme->status : 0, 0, 0);
   }

and instead adds this one:

db_query("INSERT INTO {system} (filename,name,type,owner,bootstrap,info) VALUES ('%s','%s','%s','%s',%d,'%s')",
        $file->filename,
        $file->name,
        $type,
        isset($file->owner) ? $file->owner : '',
        isset($file->bootstrap) ? $file->bootstrap : 0,
        $file->info);

Notice in the original it's inserting 'theme' directly, while in the new version it's replaced with '$type'. I don't see where $type gets set, though, so I suspect that's what the problem is.

AlexisWilke’s picture

Drupal 6.15 fixes this problem:

1. DELETE FROM system WHERE type = 'theme'

2. INSERT themes INTO system

I wonder why it took so long to get that in there! 8-)

Anyway, I think this bug is moot with 6.15. Did you try that newer version?

Alexis

bcobin’s picture

I am sorry to report:

I hadn't seen this problem in a while and saw it today with 6.15 - this was an upgrade from 6.14, not a clean 6.15 install.

So I would say this bug isn't squished just yet... I'll be keeping an eye out...

zarko’s picture

subscribing

protoplasm’s picture

subscribing, problem occasionally exists in 6.15

cels’s picture

subscribing

thaddeusmt’s picture

Perhaps it's because I'm not running cron while I'm developing, but I still get this error periodically in 6.15 as well. Subscribing.

AlexisWilke’s picture

I still did not get the error since I got 6.15 installed...

protoplasm’s picture

Just happened again after accessing the performance page.

user warning: Duplicate entry 'themes/garland/garland.info' for key 1 query: INSERT INTO system (name, owner, info, type, filename, status, throttle, bootstrap) VALUES ('garland', 'themes/engines/phptemplate/phptemplate.engine', 'a:13:{s:4:\"name\";s:7:\"Garland\";s:11:\"description\";s:66:\"Tableless, recolorable, multi-column, fluid width theme (default).\";s:7:\"version\";s:4:\"6.15\";s:4:\"core\";s:3:\"6.x\";s:6:\"engine\";s:11:\"phptemplate\";s:11:\"stylesheets\";a:2:{s:3:\"all\";a:1:{s:9:\"style.css\";s:24:\"themes/garland/style.css\";}s:5:\"print\";a:1:{s:9:\"print.css\";s:24:\"themes/garland/print.css\";}}s:7:\"regions\";a:7:{s:6:\"header\";s:6:\"Header\";s:4:\"left\";s:12:\"Left Sidebar\";s:5:\"right\";s:13:\"Right Sidebar\";s:11:\"content_top\";s:11:\"Content Top\";s:7:\"content\";s:7:\"Content\";s:14:\"content_bottom\";s:14:\"Content Bottom\";s:6:\"footer\";s:6:\"Footer\";}s:7:\"project\";s:6:\"drupal\";s:9:\"datestamp\";s:10:\"1260996916\";s:8:\"features\";a:10:{i:0;s:20:\"comment_user_picture\";i:1;s:7:\"favicon\";i:2;s:7:\"mission\";i:3;s:4:\"logo\";i:4;s:4:\"name\";i:5;s:17:\"node_user_picture\";i:6;s:6:\"search\";i:7;s:6:\"slogan\";i:8;s:13:\"primary_links\";i:9;s:15:\"secondary_links\";}s:7:\"scripts\";a:1:{s:9:\"script.js\";s:24:\"themes/garland/script.js\";}s:10:\"screenshot\";s:29:\"themes/garland/screenshot.png\";s:3:\"php\";s:5:\"4.3.5\";}', 'theme', 'themes/garland/garland.info', 1, 0, 0) in ...modules/system/system.module on line 822.

Danny_Joris’s picture

Subscribing. I have the same issue.

sangus’s picture

Subscribing.

joshmiller’s picture

Hate to be a "me too," but I confirm in two very different, well-maintained (Drupal 6.15 + all modules up to date), cron run sites I'm experiencing this error. Can anyone accurately describe what the problem is?

If we can describe it, then we can fix it.

I found this issue by googling: Duplicate entry 'themes/pushbutton/pushbutton.info'

Who uses pushbutton anyway ;P

Josh

Sborsody’s picture

I created a "siteadmin" role awhile ago and gave this role the "select different theme" permission. At some point I created a user and gave that user the "siteadmin" role and suddenly this error started occurring. I haven't seen hide nor hair of this error since I removed the "select different theme" permission. No role has this permission and on my site it isn't needed anyway (theme can be changed using Administrator account). Maybe that would work for others too.

protoplasm’s picture

...best I have observed so far, it seems to occur on a RARE occasion (maybe once or twice a week when I am working on the site) in three different instances: 1) with a slow load (cause or result?), 2) after a manual cron job, or 3) when accessing the performance page. That having been said, it does not occur (fingers crossed) on my production site--only my remote sandbox--which, in theory, is nearly identical in every way with my live site--same server, software, etc. The site is fed the same database and public_html as the production site. Only differences are .htaccess and settings.php--and those changes are only site specific (different domain name and passwords). It makes no sense to me at all. I do have switchtheme configured. The two themes are necessary. Fluid for widescreens and fixed for iphone and more traditional dimensions.

mikeytown2’s picture

There's a race condition in system_theme_data()
http://api.drupal.org/api/function/system_theme_data/6

Look at the code and it's quite obvious as to why. One way to minimize this would be to use the db_multi_insert patch that I've been working on for the menu_router; but the correct way is to implement the locking patch as that fixes the main issue. db_multi_insert will greatly reduce the chance of this occurring.
#512962: Optimize menu_router_build() / _menu_router_save()
#251792: Implement a locking framework for long operations

Alan.Guggenheim’s picture

I am running 6.15. It is occurring more and more on my site. Now several times a day for each user, with a screen full (actually scrolling down sevral screens of red error messages). We need some solution or temporary work around on this, please.

I looked into my Themes at http:///admin/build/themes, and none of the themes have the enabled box checked!

Do you recommend removing all themes?
subscribing

AlexisWilke’s picture

Alan,

In general, when I re-select the themes I want to use on my site, the errors go away (till next time it gets lost.)

But these days, it has worked great for me.

Try that at least to temporarily fix your errors.

Thank you.
Alexis Wilke

escoles’s picture

On one site where I was having this error under 6.15, I found that it only cited one theme (which I wasn't using and which wasn't enabled). Removing the directory for that theme caused the error to stop on that site.

I'm still seeing it on another site and unfortunately, the errors reference the default theme. Fortunately the error doesn't occur as often on that site. Unfortunately, it's one I'm about to hand over to a client, and I'll have to explain to them that it's merely cosmetic.....

AlexisWilke’s picture

Note that there is a huge problem in 6.15 since the first step is:

DELETE FROM system WHERE type = 'theme';

And that's why we lose the selection...

Although the fix helps greatly in some respect, it blows away your theme selection. (i.e. the status)

Yet, it works a lot better now for me than it was before.

bstrange’s picture

OK, this is really strange my client has a D5 site that I built for them a while back and it has always ran well. They were interested in upgrading to D6 and accomplishing some things that I have implemented on another D6 site, so I created a sub domain and built a 'test' site as s subdomain of the D5 site. The sub worked great and everything looked good; so I updated the main site to D6 and the upgrade looked great. I pulled up the subdomain and the front page featuring the DDBlock module had gone crazy, (this should have been a clue that there was something wrong with the theme layer as ddblock utilized preprocess functions controlled by the theme's template.php file), but since the sub was now just an effective 'sandbox' for the main site and all I was going to be doing was pull views and content types up to the primary domain I ignored it.

I opened up content types, chose one of the custom types, hit manage fields and was hit with the error that everyone here is very familiar with, so in the interest of space, I won't repost it. Remembering the first indication was the corrupted ddblock, I decided to try the ddblock troubleshooting steps, chose a different theme, then rechose my first with no effect; then, I went to performance and cleared the cache... then went to update.php...then plugged the error into google and found this issue and the plethora of duplicates.

Seeing how no one knows to know what causes it, and the developers seem disinterested in the issue as a whole. I mean, we know what part of the code causes it (I *think* - I'm not a developer and am not even comfortable editing the sql database, though I occasionally do, and I don't understand most of this page) but we don't know why it seems to happen out of the blue. So instead of posting the code that I don't understand, I figured I'd post what led up to it and the interesting occurance (to follow) that I have experienced since.

While cobbling around my site and looking at this issue fairly confident that a fix for my 6.15 issue doesn't exist as an easily applicable solution, I enabled one of my other custom themes. I have 2 custom themes, one a (1)Garland mod in sites/all and the other a (2)Garland mod that is partially in sites/all and partially in sites/default/files/color/customthemename due to utilizing changes made by the Garland color module. So I enabled the (2)Garland mod that exists in sites/all as well as sites/default/files/color... and low and behold, the ddblock on the front page was fixed, no error when accessing content types, views, clearing cache, or running update.php... so I thought hmm maybe it went and 'fixed' itself as others have mentioned, so I renabled my main theme (the one simply in sites/all) and nope... ddblock still messed up, cleared cache, ran update php, accessed content types, all throwing errors. Switched back to the other Garland and no errors...

To me this seems interesting, both due to the location of the custom file differences, and because of the ddblock preprocess errors occurring on one and not the other.

Whatever the case, I really hope to hear of a fix that those of us who aren't experts can perform sometime in the future, because I am sure, with my luck (considering it's a client's site) that I will experience this on the main site eventually, though I pray that I don't :)

Dave Reid’s picture

Now that #251792: Implement a locking framework for long operations is in D6, we should use that to fix this instead of changing APIs.

AlexisWilke’s picture

bstrange,

You probably have some mismatches because of the change of domain name sub.example.com -> www.example.com can cause problems.

One way to fix the problem is to keep sub.example.com working...

Alexis

bstrange’s picture

Yes, a 'normal' person would have built the new site in a subdomain sandbox or a local sandbox (though I couldn't use local because the client needed to be able to view it and request changes) and then move it into the main domain, but I like to make things more complicated than they should be :P

I actually built the new site and all the views, content types, display blocks, & panels in the sub and then simply did a full D5-D6 upgrade on the parent and then exported the views, content etc into the main and coppied the theme directly. This wasn't entirely because I like making more work for myself as there was a ton of content on the main domain and I needed to easily preserve it.

There (knock on wood) seem to be no problem with this in the primary domain. In the sub domain, which remains intact and unaltered, the error occurs, but only in one of the two custom themes, where it is a near constant corruption of the theme layer and the log produces the 'race condition' errors when accessing any content type. If I switch to the theme that exists in both sites/all and sites/default/files/color/customtheme the error does not occur in any fashion (at least hasn't to this point).

Once I finish copying content over, I will switch to one of the 5 default themes and see if the race condition occurs in those as well. It may be a red herring, but I am interested to learn if the race condition effects only the theme that it first happened in as well as if the one that houses it's style.css in sites/default/files/color/customtheme is 'immune' to the race effect because of it's unique style.css position due to the color module. It seems illogical that would be the case as the themename.info is what tells the system that the theme is installed, and even the theme whose style.css is located outside sites/all has a themename.info in the standard sites/all/themes...

As I said before, I am a cobbler, and know very little about coding, php functions, and Drupal's core mechanics, but I have found that it is usually the similarities or anomolies that lead me (or others) to discover a root cause of a situation.

Thanks for the reply, and I apologize if I am polluting the issue with useless banter (as I am unsure if any of my posts on this matter contain even semi relevant information at this point). Say the word and I'll stop posting my experiences and leave the thread to the experts :)

AlexisWilke’s picture

Status: Needs work » Needs review
FileSize
740 bytes

There is a very simple patch that prevents anyone from accessing the system table while you're messing around with it in the system_theme_data() function. The idea is very simple: I lock the table.

I'm not too sure why some people think they should never ever do that. That's the first thing you learn about database systems that for consistency you need to lock tables... Anyway, if you do DELETE FROM system WHERE type = 'theme', the table is completely messed up. So hoping that someone else can do something with it is futile. At least, not until you're done rebuilding the table. The lock will prevent use of the table while it is not valid.

Good luck! (And yes, I got the error once yesterday so I thought I could do a little patch...)
Alexis

Dave Reid’s picture

Status: Needs review » Needs work

Please write a patch using the locking framework: #251792: Implement a locking framework for long operations.

mikeytown2’s picture

AlexisWilke’s picture

Guys,

I'm being cheap... but unless someone pays me to implement a 1,000 line fix, no way it is going to happen here.

On my end, I'm just trying to help those who cannot write even one line of code. A quick fix is worth a lot more than no fix. 8-) (ask them!)

Good luck!
Alexis

P.S. By the way, if the users have 1,000 themes, it may be a big problem, they could also delete ALL the themes that they do not need (except Garland which is necessary for updates, and just in case) In other words, that lock would be done for 2 maybe 3 themes for most people... Not a killer!

mikeytown2’s picture

Status: Needs work » Needs review
FileSize
7.98 KB
AdrianB’s picture

Subscribing.

xerexes’s picture

Does anyone understand the conditions for why this duplicate error happens? Clearly it doesn't impact every Drupal installation - it just started happening to my site (comixtalk.com) sometime last night (version 6.15. Is there a particular configuration, type of activity that causes it?

I tried rolling my system table back to a version from last week - no dice, still getting the issues. It seems to happen to every user/viewer on a bunch of posts and pages - once per page, whether logged in or not.

Is it possible to roll back the entire database to avoid this? Is there something about a particular server setup that is at issue? This is actually a big problem for me and I'm surprised the support to date is as murky as it is (unless this is really a pretty rare problem) -- shd I hop on IRC? Has anyone trouble-shot this for a client?

Not trying to whine - just want to fix and other than the hacking a core module approach in #80, not seeing anything offered that actually purports to solve the problem for a production/live site. Am I missing anything?

AlexisWilke’s picture

xerexes,

Try #80 and let us know if that solves your problem. The lock is only for you few themes, if you have extra themes that are not used, delete them if you want to make sure you get the full speed.

I rarely get this error since Drupal 6.15. It is a concurrency problem, although I'm surprised you get the error when anonymous users browse your site.

Thank you.
Alexis

escoles’s picture

If I follow the discussion correctly, #80 is a smaller patch (adds just 4 lines), but #84 is a more canonical patch (uses the API).

The more canonical approach might be more stable. OTOH, until it's tested, because it changes more lines of code it may pose a greater risk (i.e., only people operating at or above the skill-level of mikeytown2 et al will be able to evaluate its riskiness by reading through it).

Personally I'm thankful for both the contributions. For myself, I'd probably make the smaller patch (because I can understand it), at least until #84 is tested. Fortunately, I've been able to address the issues in 6.15 by removing unused themes.

mikeytown2’s picture

I rolled out #84 on the quick since AlexisWilke thought it would be a 1,000 line patch. It's there to kinda prove that the new API is not impossible. Hopefully I'll get some time to test it out this week.

srobert72’s picture

Subscribing

Jerome F’s picture

Subscribing

The patch in #80 seems to have done the trick for me. I know that means patching drupal core, but this error message was pulling my nerves. And I'm glad to get rid of it whatever the means.

Thank you AlexisWilke

Is this safe on a production website ?

I'm not a dev at all, but I don't see why locking a table would not be safe. I understand that patching the core is not that good for upcoming updates though.

sapiusenator’s picture

Disabling the skinr module made this go away for me, re-enabling it made it come back immediately. All latest versions of modules and core as of this date. moving to proper page, this is for "User warning: Duplicate entry... ...themes error" that this fix will help

AlexisWilke’s picture

Status: Closed (duplicate) » Needs review

Jerome,

Thank you for reporting your experience.

It is actually safer for any website to do a lock of tables they update. In my case, the LOCK happens too late, it should be before the SELECT... Just trying to save a little time... The concern is how long the lock lasts since it prevents concurrent accesses. Large businesses often use different set of databases to palliate. Some are read-only and some are read-write. That way, they can manage really large numbers of hits without penalty.

However, in this case, if you have 10 to 20 themes, the lock will be really short. #84 is a much better approach since it prevents a lock of the entire table and it prevents a double refresh of the table (with the same data each time.) Of course, if #84 is not properly tested before 6.16, it would be neat (to avoid many errors on many sites) to have #80 applied in the meantime. The main difference will be speed on systems that run a large number of themes.

Alexis

Jerome F’s picture

@AlexisWilke

Many thanks. I assume that as I'm going to run only 6 themes once I'm done with my multisites, #80 will be ok for me. I keep that in mind and I'm looking forward to the evolution of this issue.

BrightBold’s picture

With the patch from #84, I get the following error:

Fatal error: Call to undefined function lock_acquire() in \includes\module.inc on line 97

Has anyone else tested? I patched by hand (not a developer - need to get some patching tools!) so I could have made an error, but I did double-check. Also, that patch appears to include some unnecessary changes (lines removed and replaced by identical lines) in the later parts of the patch. (Not a big deal, but presumably should be cleaned up if I've interpreted correctly.)

I'll test the patch from #80 next.

mikeytown2’s picture

@BrightBold
Test with 6.x DEV of drupal

GiorgosK’s picture

tested #80 patch and it seems to clear the warnings
when I get a change I will test #84 patch since it needs the drupal dev version installed

jrtorrent’s picture

Thak you, thank you, ... THANK YOU. Tested #80 and works like a charm.

Now if I could find a solution to a similar problem with CCK (Duplicate entry 'sites/all/modules/cck/modules/text/text.module' for key 1 query: INSERT ... and so on) I would be in heaven (maybe that's a little too high but right now sleeping for a full night would be almost as good!-).

Regards

JRT

BrightBold’s picture

@mikeytown2 - the patch in #80 worked with 6.15, and it seems to my admittedly limited understanding that the duplicate entry issue that was plaguing me will likely be fixed in 6.16 with #251792: Implement a locking framework for long operations. If the problem isn't fixed I'll come back and try your patch on 6.16. Sorry not to be of more help - I'm not a developer so I don't have a 6.x-dev sandbox set up to play with.

jannalexx’s picture

Subscribing

stijnbe’s picture

Version: 6.x-dev » 6.16

The patch in #80 worked for me on 6.16. The error is gone. But sometimes when I go to the administrator pages (modules list, themes,...) there is a timeout.

montag’s picture

subscribing

jibbajabba’s picture

subscribing. (sorry)

mikeytown2’s picture

FileSize
5.76 KB

This one works, please test so we can get this RTBC.

Jerome F’s picture

Patch in #104 works for me

escoles’s picture

I applied patch @ 104 against D6.16, and it seems to have no effect on the problematic behavior on the one site where I'm still seeing it on a predictable basis. Frequency is unaffected. However I think I can say when I'm seeing it, approximately: When I restore multiple browser tabs displaying admin screens. In the most recent case, there were three views export screens, the main Views listing page, and the admin page for the CKEditor module. The errors were showing up on two of the Views export pages.

mikeytown2’s picture

@escoles
does patch #80 fix this issue for you? If not then it might be a different issue
http://drupal.org/node/550254#comment-1928800
http://drupal.org/node/512962#comment-2819410

escoles’s picture

Oy. OK, I'll check that tonight to see if #80 works. I applied #80 against some other sites which were exhibiting what looked to me like really similar behavior, and it seemed to work on those sites. That was against D6.14, I think. As I recall it was quite short so it should not be difficult to manually apply against D6.16.

Rob T’s picture

I just started getting this error as well following an upgrade from 6.13 to 6.16. Crazy annoying.

bcobin’s picture

Saw this error the other day on a production site - has this been "officially" solved for 6.16? Reluctant to apply a patch unless/until there's a solution that seems to work. Is #104 the current best bet? Thanks!

mikeytown2’s picture

#104 is the best bet as it only updates the database once if multiple requests come in; this works for me. #80 is known to work but it will update the database multiple times, resulting in long page loads since it waits for the db lock to be broken. Also there are no calls to db_lock_table() in core so I doubt #80 will ever be accepted into core.

Ron_WI’s picture

Subscribing

valante’s picture

#104 worked for me against D6.16 . Load time reduced from over a minute to a few seconds.

bcobin’s picture

#104 Patch applied successfully on two production sites and one development site - load time also seems to be significantly reduced; is this to be expected? In any event, nice work.

I'll report back should I see the problem arise again - I am most grateful... thanks much!

ar-jan’s picture

Applied #104 on a production site about a day and a half ago. Drupal 6.16. Haven't had any more of those user warnings, so it worked for me. No other effects noted.

mikeytown2’s picture

Status: Needs review » Reviewed & tested by the community
Dig1’s picture

Hi Mikeytown2

I am running D6.16 and ran into the problem 'user warning: Duplicate entry 'themes/garland/garland.info' for key 1 query:...' for the first time ever (in 2 Drupal years) last week.

I found this node and applied your patch from #104.

The good news it that the problem has not reappeared.

Thanks

Dig1

kenorb’s picture

+1

acrollet’s picture

subscribing

mariano.barcia’s picture

Sorry, but I cannot confirm the patch in #104 is working for me. Under D6.16, I applied the patch, and made no difference at all, except for the fastest load time of the modules page, which is noticeable. Am I missing any step? (clear the cache for example?). Problem is that I can't get drush to do anything...

mikeytown2’s picture

it appears that #104 fixes a lot of issues, but it seems like something else is causing issues.

@mariano.barcia
What bug are you experiencing? Describe your situation.

mariano.barcia’s picture

Just in case, I cleared the cache, to no avail. So, I double checked the patch was applied (just in case again!), and then renamed the module's files and functions to a temporary name and the error went away. Reverted the rename operation and everything seemed to be working fine, but oh no, same problem again. BTW, the module giving trouble is a custom module, and I still haven't double checked the correct declarations in the .info file... Not sure what to do now... perhaps delete the entry in the system table? I will keep you posted.

mariano.barcia’s picture

After deleting the entry of the module in the system table, re-enabled the module, and the error went away.

gooddesignusa’s picture

subscribing....
I get this error randomly. Sometimes after messing with views or on the reports pages If I remember correctly. I've seen this happen on multipule sites i've worked on.
On average I might see it once or twice a week on a site i'm working on just by myself. (closed to public)

mikeytown2’s picture

@gooddesignusa
How does the patch from #104 work for you?

gooddesignusa’s picture

Once it happens again I will try it out. So at least I can try to recreate it to see if this patch helps lol.

Dig1’s picture

Yes I had to use patch from #104 again on another new 6.16 site and it worked again.

Cheers

Dig1’s picture

Yes I had to use patch from #104 again on another new 6.16 site and it worked again.

Cheers

bleen’s picture

Subscribing

mikeytown2’s picture

For those of you still getting "Duplicate entry" errors after applying the patch in #104 I have another patch for you to test
#561990: Avoid variable_set() and variable_del() stampedes

pwolanin’s picture

Version: 6.16 » 6.x-dev
Status: Reviewed & tested by the community » Needs work

needs work - there is no use of lock_wait() or at the least, code comments explaining what the behavior or results is for a parallel thread that fails to acquire the lock. Does it make sense for those parallel threads to proceed while these rebuilds are happening?

If it got into D7 this way, then that needs to be fixed.

compare the use of locking in menu and locale.

mikeytown2’s picture

In this case it does make sense for the parallel threads to proceed. Why rewrite the same data to the database 5 times if the same page got loaded 5 times? Only need to do it once.

menu_rebuild is an action that mainly saves the data to the db; thats what it's supposed to do thus it makes sense to lock till that operation is done; returns TRUE or FALSE and most if not all functions that call it do nothing with the return value. If it can't get a lock it waits for the other operation to complete & then does no writing. If for some strange 1 off reason the 2nd menu_rebuild call has different data then the first, the newer data will not get saved. Not an issue since the odds of this happening are very small.

local if I'm reading it right appears to skip if it can't get a lock, similar to what I'm doing here. Makes sense since it's returning a value that gets used. Does a DB read with a cache write in the lock. locale does not use lock_wait.

I will add comments to the patch explaining the logic and code flow.

mikeytown2’s picture

Status: Needs work » Needs review
FileSize
5.86 KB

Added in a comment in the correct locations.

donquixote’s picture

Could you guys check if the following is related, or even fixes the bug?
#632080: _system_theme_data() should clone the returned objects.
It is just a guess, because I can't remember seeing this bug on my patched sites.

pwolanin’s picture

#147000: Rewrite module_rebuild_cache() and system_theme_data() is the correct issue to be working with, as Damien pointed out above. This is a duplicate.

jbrauer’s picture

Status: Needs review » Closed (duplicate)
mcurry’s picture

*subscribe*

calefilm’s picture

Status: Needs review » Closed (duplicate)

mikeytown2,

I've been updating my progress with my battle with "Duplicate Entry...." for a year and a half now. I just found your patch and applied it. It succeeded and so far I don't have my multpile Duplicate Entry errors. It's been nearly 15 minutes and that's a good sign. yeah. I had it that bad. I really appreciate you posting this. If it works I'll tell all my friends! :P

UPDATE: I don't receive anymore errors, however, I'm still losing my Enabled Themes... The default theme stays but any Enabled Themes automatically disable themselves after a couple days of work.. don't know when they get disabled. I use PAGE THEME module to keep all my themes running even when they are disabled. Works. And no more errors in Log Entries.

I just have two questions..

1) Should I be worried about not being able to apply this patch in future versions of Drupal?
2) The posts above me says this is a duplicate issue. Is that thread working on this same issue? Because this patch worked for me.

Chris-Ju’s picture

How to get the patch #147000 mentioned in #135 and #136 ?

mattew’s picture

Updated the patch given in #133 for Drupal 6.22.
Works for me, please give feedback if needed.

petertj’s picture

subscribing... will try the patch.

Anonymous’s picture

Can you rewrite the patch for D 6.24? Thank you!

mvc’s picture

yes, this is a duplicate.

#147000: Rewrite module_rebuild_cache() and system_theme_data() was for D7 and is marked fixed.
#1425868: Duplicate entry of themes primary key in systems table of Drupal 6.24 (using Drush or Ægir) is where this discussion is happening now. please go help test the latest patch there.