Node Access Rebuild never finishes (infinite loop)
PaulMagrath - September 30, 2008 - 14:49
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | node.module |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs work |
Description
Small bug in the core "node" module's node.module
Current code:
<?php
function _node_access_rebuild_batch_operation(&$context) {
....
while ($row = db_fetch_array($result)) {
$loaded_node = node_load($row['nid'], NULL, TRUE);
// To preserve database integrity, only aquire grants if the node
// loads successfully.
if (!empty($loaded_node)) {
node_access_acquire_grants($loaded_node);
}
$context['sandbox']['progress']++;
$context['sandbox']['current_node'] = $loaded_node->nid;
}
...
}
?>The last line in this while loop should read:
<?php
$context['sandbox']['current_node'] = $row['nid'];
?>As if the loaded is empty, $loaded_node->nid will be empty too causing an infinite loop in the batch operation, as it takes an empty value for $context['sandbox']['current_node'] as meaning it has not yet started the rebuild and starts all over again.
#1
We can only review patch files. Since there is no file marking as active.
#2
+1 this fix, I've encountered the same error and it was resolved by changing that one line.
#3
I believe this fixes a problem that almost has had me pulling my hair out. I've rebuilt permissions a hundred times before and never had it stall or consistently keep saying 'The content access permissions have not been properly rebuilt.' Disabling and uninstalling modules hasn't fixed it on the site suffering from this problem so I dove into the batch process. It's easy to see that if not all nodes are processed in _node_access_rebuild_batch_operation() than the batch process reports it as unfinished and node_access_needs_rebuild(FALSE) is never called. What's difficult for me to deduce at this moment is why the node_load() may not return a full node object. However, since it's just wrong to use $loaded_node->nid in the case it might not be an actual node object we really should be using the $row['nid'].
#4
This is likely a problem in HEAD as well http://api.drupal.org/api/function/_node_access_rebuild_batch_operation/7 because if $node is empty then $context['sandbox']['current_node'] won't record the processed node id.
Edit: Spoke with Dave Reid on irc about this and node_load_multiple() doesn't return any invalid nodes so this bug probably won't occur in 7.
#5
The fix makes complete sense, nice catch. I guess such cases, where there is a record in the node table but node_load() fails, can happen with deleted node types.
D7 can be affected too, though : if *no* valid node is found within the nid range, then $context['sandbox']['current_node'] won't be updated, and infinite loop ensues.
So, patch in #3 is RTBC for D6, but this will need to be fixed in D7 first. Here's a patch, needs review.
#6
The last submitted patch failed testing.