I'm not sure if the following issue is the expected behaviour, but I had some code affected by that which prevented the right node submission (throwing some errors: #1627860: Unable to add new content with empty upgraded image/file field #6 and doing some really really weird stuff).

Following simple code is explaining the problem:

  $node_in = new stdClass(); // We create a new node object
  $node_in->type = 'page'; // We set content type
  ctools_object_cache_set('test', 'node', $node_in);
  $test = ctools_object_cache_get('test', 'node');
  $test->type = 'overridden';
  $node_out = ctools_object_cache_get('test', 'node');
  var_dump($node_out); exit;

returns:

string(9) "overridden"

drush version:


$ drush eval "$node_in = new stdClass(); $node_in->type = 'page'; ctools_object_cache_set('test', 'node', $node_in); $test = ctools_object_cache_get('test', 'node'); $test->type = 'overridden'; $node_out = ctools_object_cache_get('test', 'node'); var_dump($node_out); exit;"
Drush command terminated abnormally due to an unrecoverable error.                                                                                                                             [error]
object(stdClass)#230 (1) {
  ["type"]=>
  string(9) "overridden"
}

It seems that once variable is cached by CTools, the value/structure can be overridden using some local function.
In my case the function which was overriding my cached object was _field_invoke_multiple() when doing node preview in my multistep form.
See: #1622952: The right way to programatically render the preview of node object before the creation #3
More details about my problem:
http://drupal.org/node/1627860#comment-6204076

Following code can solve the problem when getting the cached object:

  $test = clone ctools_object_cache_get('test', 'node');

or to clone returning object in ctools_object_cache_get()

Comments

kenorb’s picture

StatusFileSize
new7.54 KB

Wrong patch.

kenorb’s picture

Status: Active » Needs review
StatusFileSize
new594 bytes

Proposed patch.

Status: Needs review » Needs work

The last submitted patch, includes-object-cache.inc-1676696.patch, failed testing.

kenorb’s picture

StatusFileSize
new441 bytes
kenorb’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 1676696.patch, failed testing.

sdboyer’s picture

Status: Needs work » Needs review

yep. this is how php works - since php5, the value stored in a variable is the object accessor, not the object itself. when that variable is passed by value, the accessor is copied, but both accessors still point to the same object instance.

i see no problem with cloning the cache objects that are generated. it is a bit of an API change, but it seems more reasonable for those who utilize this API to expect that the objects will not be tied to each other in memory. the database is the canonical version, always and only, and since ctools_object_cache_set() clears the static hashmap of all cache entries (perhaps overkill) whenever it is called, it is probably sanest for the API that we ensure ctools_object_cache_get() only ever spits back what's in the db.

i'm running tests on this now (since testbot failed), but will likely commit presently.

sdboyer’s picture

Status: Needs review » Needs work

oh, duh. this fails tests because nothing about the API guarantees the type of the returned data. no guarantee that it'll be an object, and you can't clone non-objects.

revise the patch to accommodate a non-object return value, and i'll commit.

greggles’s picture

Issue summary: View changes

added returns

kenorb’s picture

Priority: Normal » Minor
avpaderno’s picture

What should the returned value be, in the case $cache[$key] isn't an object?

avpaderno’s picture

Status: Needs work » Needs review
StatusFileSize
new580 bytes
avpaderno’s picture

StatusFileSize
new601 bytes

(I am not used to VIM for editing PHP files.)

avpaderno’s picture

StatusFileSize
new600 bytes

(Sigh!)