Problem/Motivation

Menu rebuild causes:
Object of class stdClass could not be converted to int in _menu_router_build() - line 3370
error when using some contrib modules such as nodequeue.

This is due to a bug in the behavior of array_multisort(), which is used in _menu_navigation_links_rebuild(). array_multisort() keeps sorting by all dimensions of the array, and if any of the (sub)-children are objects it will trigger a notice that it cannot compare them to other types.

Here is a generic PHP script that will trigger this error:

<?php
$sort
['A'] = 2;
$sort['B'] = 1;
$sort['C'] = 1;
$menu['A'] = 0;
$menu['B'] = (object) array(0);
$menu['C'] = 0;
array_multisort($sort, SORT_NUMERIC, $menu);
?>
(See also the PHP bug report.)

Proposed resolution

Fix the sorting of menu items to not use array_multisort() but just a normal asort(). A patch implementing this change is in #972536-63: Object of class stdClass could not be converted to int in _menu_router_build() - line 3370. A test that reproduces the bug is included in the patch.

A workaround for modules that trigger this error is to change the order of arguments in hook_menu() declarations. See the the commit to fix this bug in the nodequeue module.

Remaining tasks

User interface changes

None.

API changes

None.

Original report by tekante

When doing a minimal install with Drupal 7 Beta2, the above error is observed after enabling the nodequeue module when using the head revision and patches http://drupal.org/files/issues/nodequeue-d7-db-insert-errors-fix.patch and http://drupal.org/files/issues/nodequeue-941558-16.patch.

Why am I not filing this in the nodequeue queue? I don't believe the error is due to anything nodequeue is doing wrong based on the variety of things that resolve the error. Comparing its info to that of another item (admin/structure/types does not show any obvious errors). There is an issue filed related to this but I wanted to bring it to the attention of the menu system maintainer [959904]

Attached is a file containing a serialized dump of the $sort and $menu states along with comments about various manipulations to the data that result in the error disappearing.

The error disappears in the following conditions:

1. Unsetting arbitrary admin/structure/nodequeue menu entry sub keys
2. Adding a SORT_STRING parameter as the 4th argument to the array_multisort call on line 3370
3. Unsetting an arbitrary subkey of another menu entry (admin/structure/types in the attached file)

My PHP versions are
PHP 5.2.13 (cli) (built: Mar 7 2010 09:15:25)
and
PHP 5.3.2 (cli) (built: Aug 7 2010 00:04:41)

Files: 
CommentFileSizeAuthor
#86 drupal-menu-int-972536-86.patch1.37 KBroma7
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-menu-int-972536-86.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#83 drupal-menu-int-972536-83-D7.patch3.06 KBjamsilver
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-menu-int-972536-83-D7.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#82 drupal-menu-int-972536-81.d7.patch3.28 KBjamsilver
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-menu-int-972536-81.d7.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#81 menu_router_build-972536-POST-APOCALYPSE.patch3.34 KBxjm
PASSED: [[SimpleTest]]: [MySQL] 33,759 pass(es).
[ View ]
#78 drupal-menu-int-972536-78.patch3.3 KBSteven Jones
PASSED: [[SimpleTest]]: [MySQL] 33,630 pass(es).
[ View ]
#75 drupal-menu-int-972536-75.patch2.98 KBSteven Jones
FAILED: [[SimpleTest]]: [MySQL] 9,652 pass(es), 0 fail(s), and 160,284 exception(es).
[ View ]
#63 972536-63.patch2.88 KBchx
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 972536-63.patch.
[ View ]
#62 chx_array_multisort.txt258 bytesbfroehle
#60 array_multisort_test.txt175 byteschx
#60 972536_60.patch1.69 KBchx
FAILED: [[SimpleTest]]: [MySQL] 30,082 pass(es), 0 fail(s), and 412 exception(es).
[ View ]
#52 972536-52.patch2.02 KBJerome F
FAILED: [[SimpleTest]]: [MySQL] 29,423 pass(es), 0 fail(s), and 210 exception(es).
[ View ]
#54 972536-24.patch3.1 KBbfroehle
PASSED: [[SimpleTest]]: [MySQL] 29,402 pass(es).
[ View ]
#24 972536-24.patch2.63 KBSteven Jones
PASSED: [[SimpleTest]]: [MySQL] 29,393 pass(es).
[ View ]
#23 972536-23.patch1.69 KBSteven Jones
FAILED: [[SimpleTest]]: [MySQL] 29,889 pass(es), 274 fail(s), and 401 exception(es).
[ View ]
#10 972536-10.patch528 bytesbfroehle
PASSED: [[SimpleTest]]: [MySQL] 31,542 pass(es).
[ View ]
#3 Notice.pdf520.03 KBWhiplashInfo
#1 object_conversion_menu_router_build-972536-1.patch437 bytestekante
PASSED: [[SimpleTest]]: [MySQL] 28,957 pass(es).
[ View ]
multisort.php_.gz12.27 KBtekante

Comments

StatusFileSize
new437 bytes
PASSED: [[SimpleTest]]: [MySQL] 28,957 pass(es).
[ View ]

Patch attached adds the SORT_STRING parameter which causes the error to stop occurring for me. Note that this may just be covering up the true cause of the error.

Hopefully, to help narrow down the problem, I noticed that the error only occurred when the 'Smartqueue taxonomy' and 'Nodequeue generate' Module-elements were enabled. As soon as I disabled them the problem disappeared.

I was adding a new Content Type when I originally noticed the error.

Version:7.0-beta2» 7.0-rc1
StatusFileSize
new520.03 KB

I have 7 RC1 installed and when enabling entity and rules modules the Notice: Object of class stdClass could not be converted to int in _menu_router_build() (line 3443 of /home/web25518/domains/fmedk.se/public_html/d7/includes/menu.inc occured.
Se my attached file.

Thanks / Tomas

Status:Active» Reviewed & tested by the community

Patch works for me...

Status:Reviewed & tested by the community» Needs work
Issue tags:+Needs tests

Hm. I'd like us to have a test that exposes exactly what makes this buggy. That might also help us get to the bottom of the real bug.

Version:7.0-rc1» 7.0-rc3

I often get this when enabling/disabling modules in 7.0-rc3. So, it appears not to be just a nodeque generate or smartque issue.

Exact error: Notice: Object of class stdClass could not be converted to int in _menu_router_build() (line 3458 of /includes/menu.inc).

Version:7.0-rc3» 7.0

i having this problem with Nodequeue too. using drupal 7.0 and Nodequeue 7.x-2.x-dev.

The only thing thats of type stdClass in what we are sorting is the 'access arguments' field of $menu['admin/appearance/settings/THEME'], which comes from

<?php
 
foreach (list_themes() as $theme) {
   
$items['admin/appearance/settings/' . $theme->name] = array(
     
'access arguments' => array($theme),
...
?>

$theme here is documented to be an object (and not an array), so that's proper.

Ideally PHP would have a SORT_NONE flag so we could instead do

-  array_multisort($sort, SORT_NUMERIC, $menu);
+  array_multisort($sort, SORT_NUMERIC, $menu, SORT_NONE);

to indicate that we don't care about further sorting by $menu.

Status:Needs work» Needs review
StatusFileSize
new528 bytes
PASSED: [[SimpleTest]]: [MySQL] 31,542 pass(es).
[ View ]

Here goes nothing...

Edit: There's another similar array_multisort in _menu_navigation_links_rebuild().

#10 Patch works perfect for me.
Thanks!

Subscribing (patch in #10 helped - thanks!)

subscribing, and thanks!

If somebody is looking to write a test... the way to trigger this is to create a test module, implement hook_menu() and have some of the items return 'access arguments' or 'page arguments' that are of type stdClass. For example:

<?php
function MODULE_menu() {
 
$data = array(
   
'key' => 'value',
  );
 
$items['object1'] = array(
   
'title' => 'object1',
   
'page arguments' => (object) $data,
  );
  ...
}
?>

and see if you can get an error to trigger by just going to any page on the site.

There's a report at http://drupal.org/node/1029182#comment-4019108 (an issue which I marked duplicate of this one) that this could have something to do with disabling a module that has a customized menu item.

Still waiting on an exact set of steps to reproduce (I couldn't reproduce it myself), and it could turn out to be a red herring. But it sounds like in this issue there still isn't an understanding of the ultimate root cause of the bug, so maybe that will help.

David Rothstein: The root cause is documented in #972536-9: Object of class stdClass could not be converted to int in _menu_router_build() - line 3370. Essentially array_multisort() keeps sorting by all dimensions of the array, and if any of the (sub)-children are objects it will complain that it cannot compare them to other types.

Exact steps to reproduce are difficult, as you have to guarantee a comparison several dimensions into $menu, however the serialized version of the variables provided in the original post do give a good indication of what is going on.

Another patch which would fix the problem for the original poster (but not the inherent flaw in the sorting system) is contained in #997918-5: Menu gets rebuilt continually during install and update (and menu system stores entire, out-of-date theme objects) --- namely the changes to system.module and block.module which replace 'access arguments' with the key name instead of the whole $theme.

I have also been experiencing this. Since I don't really understand the solutions provided, except that they seem to offer only cosmetic solutions, and since I am not yet using nodequeue, my stopgap solution was to disable nodequeue. This seems to have stopped the error messages. But I am looking forward to a real solution...

Subscribing

I'm still getting this error with nodequeue.

Status:Needs review» Needs work

I think that webchick is saying in #5 that this needs tests, so marking as needing work.

Assigned:Unassigned» Steven Jones

Status:Needs work» Needs review
StatusFileSize
new1.69 KB
FAILED: [[SimpleTest]]: [MySQL] 29,889 pass(es), 274 fail(s), and 401 exception(es).
[ View ]

Okay, so what appears to be going on here is that as the menu is sorted with array_multisort(), if there any ties in the first array, then the second array is used for sorting. The second array is basically an array of menu items (arrays). This causes issues because apparently PHP will sort arrays of arrays by descending into the 'first child' of the elements to compare and then sorting again. Normally this is fine, because the keys that we have in the menu items are mostly strings and arrays, however you can pop objects in there too. PHP does not like sorting objects!

Now, normally the sorting doesn't go anywhere near the 'access arguments' property of the menu item arrays, because it looks in 'first child's first when sorting, but, nodequeue puts the access arguments property first in one of its menu items. This menu item gets sorted with some of the appearance menu items, which have objects in their access arguments. Boom!

Is this an edge case? This is the definition of edge case.

Attached is a test that adds some menu items that cause the PHP notice if Drupal is sorting incorrectly. Giving it to test bot to munch on.

Assigned:Unassigned» Steven Jones
Issue tags:+nodequeue
StatusFileSize
new2.63 KB
PASSED: [[SimpleTest]]: [MySQL] 29,393 pass(es).
[ View ]

So, this patch now takes the work from #10, and adds the test from #23 and then fixes _menu_navigation_links_rebuild() too. If this works, then it'll be a small miracle.

EDIT: If you're reading this issue then you can skip on past the testbot madness right to comment #35.

Version:7.0» 7.x-dev
Issue tags:-Needs tests+nodequeue

Status:Needs review» Needs work
Issue tags:-nodequeue

The last submitted patch, 972536-24.patch, failed testing.

Status:Needs work» Needs review

#23: 972536-23.patch queued for re-testing.

Issue tags:+nodequeue

#24: 972536-24.patch queued for re-testing.

Yay, passed!

Version:7.x-dev» 7.0

It definitely passed when set to 7.x. With your permission I'd like to switch it back to 7.0 to see if #1093682: 7.0 testing doesn't work (Major version testing doesn't work, just X.x testing?) is a valid bug...

#24: 972536-24.patch queued for re-testing.

Assigned:Steven Jones» Unassigned

@rfay, yeah, do what you want.

FYI: feature request filed with PHP for the introduction of a SORT_NONE option. http://bugs.php.net/bug.php?id=54259

(While that still would be inappropriate for our use here, it might be convenient in the future).

#24 passes OK now with version = 7.0, although it's still testing against latest. So #1093682: 7.0 testing doesn't work (Major version testing doesn't work, just X.x testing?) was a red herring. Thanks for your support :-)

+1 to marking the patch in #24 as RTBC, although I'm going to refrain from doing so since patch is due to myself in #10. Stephen Jones was wonderful enough to include tests which trigger this bug in #23 and verify that the patch in #10 solves the issue. Also included is a fix for the other use of array_multisort which might also suffer from this.

I know the array_multisort usage was originally included as a way to boost performance. I haven't looked at actual performance numbers for this, but I suspect that the performance change is negligible (or even slightly faster).

For the curious, the abstract way to trigger the bug (and the corresponding usage) is:

<?php
// Initialize $sort and $menu.
$sort['A'] = 2;
$sort['B'] = 1;
$sort['C'] = 1;
$menu['A']['access'] = array(1);
$menu['B']['access'] = array((object) array('a', 'b'));
$menu['C']['access'] = array(0);
// The next line triggers the bug.
array_multisort($sort, SORT_NUMERIC, $menu);
foreach (
$menu as $path => $item) {
 
// ...
}
?>

The proposed patch instead does

<?php
// Initialize $sort and $menu.
// ...
asort($sort);
foreach (
array_keys($sort) as $path) {
 
$item = $menu[$path];
 
// ...
}
?>

Issue tags:-nodequeue

#23: 972536-23.patch queued for re-testing.

Assigned:Steven Jones» Unassigned

#23: 972536-23.patch queued for re-testing.

#24 seems to work for me.

#24: 972536-24.patch queued for re-testing.

subscribing.

The patch in #24 stops the error appearing for me too.

Status:Needs review» Reviewed & tested by the community

Status:Reviewed & tested by the community» Needs work

I still get :

Notice : Object of class stdClass could not be converted to int in _menu_router_build() (line 3466 in (...)/Sites/acquia-drupal/includes/menu.inc).

@Jerome F please document what steps you took to reproduce the error.

Status:Needs work» Reviewed & tested by the community

I don't know actually - I tried again, reinstalled the last dev of Drupal, ran the patch and it seems all right now. I can't reproduce either.

subscribing, #24 works here

Version:7.0» 8.x-dev
Status:Reviewed & tested by the community» Needs review

D8 first. Let's get these fixed on D8 and into D7.

Issue tags:+needs backport to D7

Add a backport tag.

#24: 972536-24.patch queued for re-testing.

Status:Reviewed & tested by the community» Needs review

#24 tested again - removes the notice for me in Drupal 7.x.dev

Status:Needs review» Reviewed & tested by the community

StatusFileSize
new2.02 KB
FAILED: [[SimpleTest]]: [MySQL] 29,423 pass(es), 0 fail(s), and 210 exception(es).
[ View ]

Patch in #49 was not applied to D7 last Dev 2011-Apr-17 and is not up to date. I updated it.

EDIT see #53 - sorry for that

Status:Needs review» Needs work

JeromeF: The patch in #24 (which is the one referenced from #49) does apply cleanly to both 7.x-dev and 8.x-dev.

Also, please be careful when re-rolling patches, as in this case you omitted complete part of the patch!

Status:Needs work» Reviewed & tested by the community
StatusFileSize
new3.1 KB
PASSED: [[SimpleTest]]: [MySQL] 29,402 pass(es).
[ View ]

Please ignore the patch in #52. I've re-uploaded the patch in #24, which is the agreed upon RTBC patch.

It's also ok with last dev in D7 today.
If you please, I think it would be nice to commit this patch in D8 and then in D7, if you agree.

A side note: Are the argument parameters cached? If so, this would lead to massive performance issues with the menu cache holding potentially huge class objects (like the entire $theme cache from #9 code). Also, if these objects change latter, does the resulting code use the cached, potentially stale, object or do they reload the class instance?

So, IMHO, a primary key should be stored and the object reloaded (a wildcard loader for #9 code). And without strong arguments against, I would mark this issue as "won´t fix" and have the documentation updated.

subscribe

Subscribe.

Issue tags:-nodequeue

For everybody who subscribed here because of the nodequeue issue, I decided to fix this in the nodequeue module because I have no idea when/if this patch will make it into core.

Reference issue from the nodequeue module: #959904: Notice: Object of class stdClass could not be converted to int in _menu_router_build()

Status:Reviewed & tested by the community» Needs review
StatusFileSize
new1.69 KB
FAILED: [[SimpleTest]]: [MySQL] 30,082 pass(es), 0 fail(s), and 412 exception(es).
[ View ]
new175 bytes

I have written a little script that tries to reproduce this to no avail. So I am uploading the test from the RTBC'd patch to see whether it fails.

Status:Needs review» Needs work

The last submitted patch, 972536_60.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new258 bytes

So yes, the tests do capture the bug. :)

chx: See #35 for a simple way to reproduce the bug. I've attached it as a PHP script for your convenience.

Status:Needs review» Reviewed & tested by the community
StatusFileSize
new2.88 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 972536-63.patch.
[ View ]

Let it be then. I have simplified your reproduction script, filed http://bugs.php.net/bug.php?id=54980 because it is not a feature you need but a bugfix. Linked the bug report from a comment. That's the only change. Thanks for the reproduction script!

@chx or pwolanin
I'm asking you too directly as you are both listed as the main maintainers and will have the best knowledge to answer these two simple questions.

Is passing in and (presumably) using cached objects via the access arguments, etc a good idea? Should this be fixed?

I.e: This could result in clogging up the cache and strange bugs due to the use of stale data.

Two real life examples for the reasons why I'm concerned with this practice.

Lightbox2 had an innocence looking array of settings stored in the cck cache, but since this was based on # content types * imagecache settings * another parameter, we soon got MySQL errors on our local AND production servers due to the excessive packet size being thrown around.

I was involved in another project that mistakenly used cached objects, and we ended up with very strange errors when the trail of the cached object was obscured and it was used where it shouldn't have been, [updated and UOW pattern kicked in and saved the changes in the background]. Only a few sales were lost, but being the jewelry industry, this amounted to a few thousand dollars worth of sales missing, leading to some headaches with the book-keepers!

Alan D.: I agree we shouldn't be storing cached objects here -- the two occurrences of this in core are addressed in #997918-5: Menu gets rebuilt continually during install and update (and menu system stores entire, out-of-date theme objects) which has been committed to 8.x, but not 7.x.

However, the use of an object (generically) as an access argument isn't in any way prohibited and this patch addresses that.

chx: As far as I know,

<?php
array_multisort
($sort, SORT_NUMERIC, $menu);
?>
is interpreted as
<?php
array_multisort
($sort, SORT_ASC, SORT_NUMERIC,
               
$menu, SORT_ASC, SORT_REGULAR);
?>

This means that $menu will be recursively sorted along all of its axes. This was the impetus for my "SORT_NONE" PHP feature request -- to turn off sorting along $menu as well.

(For those not in this discussion, the patch in #63 is ready to be committed to 8.x and 7.x).

bfroehle check my bug report on php.net, the error is dependent on the first array which makes me think it's a PHP bug. Ie. the second array is not always sorted.

Aha, okay, I get it now! Thanks for the clarification chx.

I never read anything to say that the menu system could handle complex data structures. Similar to the field system that doesn't tell you that it can not handle primary keys that are not integers.

It's sad not to have a response about the potential side effects of this. I guess that I will have to manually check every module before installation for crappy practices. Even if performance issues are seen, due to changes in the uninstall process, does this mean that we must do manual db removal of the left over menu items in the menu tables too? (i.e: hook_menu() is not longer called to remove the menu items there). :(

Tagging issues not yet using summary template.

I added an issue summary.

Issue summary:View changes

Add issue summary

Issue summary:View changes

Updated issue summary.

Issue summary:View changes

Formatting.

Thanks Steven! Edit: I added a few more details about the PHP bug, related tasks, and @amateescu's workaround to the summary.

Issue summary:View changes

Updated issue summary.

Issue summary:View changes

Updated issue summary.

Issue summary:View changes

Missing period.

Issue tags:-needs backport to D7

#63: 972536-63.patch queued for re-testing.

Status:Reviewed & tested by the community» Needs work
Issue tags:+needs backport to D7

The last submitted patch, 972536-63.patch, failed testing.

Patch needs to be adjusted for changes from #1010480: Optimize _menu_navigation_links_rebuild() in _menu_navigation_links_rebuild(); the first hunk doesn't apply.

Issue summary:View changes

Added note about tests.

Status:Needs work» Needs review
StatusFileSize
new2.98 KB
FAILED: [[SimpleTest]]: [MySQL] 9,652 pass(es), 0 fail(s), and 160,284 exception(es).
[ View ]

Re-rolled for 8.x.

Performance implications of this patch:

Firstly, changes are in code that is only called when doing a menu rebuild, so this doesn't effect every page load, only when rebuilding the menu cache.
We replace two calls to array_multisort to asort but we are now essentially sorting less data, so these calls should be faster, not slower.
Instead of additionally sorting the second array, we are looping over the first array to give us the iteration order of the second. This means that we've added a call to array_keys which will use more memory, and consume some CPU. I would suspect that the effect of this is minimal though. I don't think we need a benchmark here.

Status:Needs review» Needs work

The last submitted patch, drupal-menu-int-972536-75.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new3.3 KB
PASSED: [[SimpleTest]]: [MySQL] 33,630 pass(es).
[ View ]

Ouch, that was a lot of exceptions. We'll try that again, after I've re-rolled properly.

Re: #76: As menu rebuild is plagued by performance issues that have the side effect of breaking websites, I think benchmarks are a good idea whenever we tinker with it. See #512962: Optimize menu_router_build() / _menu_router_save().

Subscribe.

StatusFileSize
new3.34 KB
PASSED: [[SimpleTest]]: [MySQL] 33,759 pass(es).
[ View ]

StatusFileSize
new3.28 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-menu-int-972536-81.d7.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Attached a backport to Drupal 7.

StatusFileSize
new3.06 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-menu-int-972536-83-D7.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

And one that works.

Status:Needs review» Needs work

The last submitted patch, drupal-menu-int-972536-83-D7.patch, failed testing.

StatusFileSize
new1.37 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-menu-int-972536-86.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Working D7 version of last patch

Status:Needs work» Needs review

Status:Needs review» Needs work
Issue tags:-needs backport to D7

The last submitted patch, drupal-menu-int-972536-86.patch, failed testing.

Issue summary:View changes

Updated issue summary.

Version:8.x-dev» 7.x-dev
Issue summary:View changes

moving back to 7.x - the relevant code is being removed in 8.x