noad_load_multiple() should return cloned objects?

alienbrain - May 4, 2009 - 18:21
Project:Drupal
Version:7.x-dev
Component:node system
Category:bug report
Priority:critical
Assigned:Unassigned
Status:duplicate
Description

Apologies if this a 'by design' known or/and wanted result. But here we go..

I've tracked this to when node_load_multiple() was introduced in #324313: Load multiple nodes and terms at once. Before, the node was being cloned before it's returned from the cache. Now it is not. This probably has the benefit of consuming less memory, but also some other nasty side effects. For example, check this code:

<?php
// Place in your drupal root, this is a file for reproducing static caching/cloning bug.
define('DRUPAL_ROOT', getcwd());
require_once
DRUPAL_ROOT . '/includes/bootstrap.inc';
drupal_bootstrap(DRUPAL_BOOTSTRAP_FULL);

// The node nid to test with, change it to any node nid you may have in the database
$nid = 1;

$node = node_load($nid);
drupal_render(node_build($node)); // > /dev/null
unset($node)

$node = node_load($nid);
// Expected: False, Result: True
var_dump(isset($node->content));
// Expected: Lots of HTML, Result: NULL.
var_dump(drupal_render(node_build($node)));
?>

#1

Damien Tournoud - May 4, 2009 - 18:48
Title:noad_load_multiple() should return cloned objects?» node_build() should be reentrant

The main issue is probably that node_build() is not reentrant. It should.

#2

alienbrain - May 4, 2009 - 19:19
Title:node_build() should be reentrant» noad_load_multiple() should return cloned objects?

But why is node_build() the main issue? :)

I was using it only as an example, here is another one:

<?php
$node
= node_load(1);
$node->foo = 'bar';
unset(
$node);

$node = node_load(1);
var_dump($node->foo); // Expected: NULL, Result: 'bar'
?>

#3

Damien Tournoud - May 4, 2009 - 19:24
Status:active» duplicate

It's the caller responsability to clone the object if it needs to be modified, see #154859: Document that objects are shared between modules.

But definitely, node_build() not being reentrant is a bad thing we should fix.

 
 

Drupal is a registered trademark of Dries Buytaert.