[drupal-devel] [bug] Make node_save() recursion-aware

Dries drupal-devel at drupal.org
Thu Aug 18 21:50:40 UTC 2005

Issue status update for 
Post a follow up: 

 Project:      Drupal
 Version:      cvs
 Component:    node system
 Category:     bug reports
 Priority:     normal
 Assigned to:  jvandyk
 Reported by:  jvandyk
 Updated by:   Dries
 Status:       patch (ready to be committed)

Won't commit.  If you are calling node_save() recursively, something is
really wrong.  For one, you can no longer guarantee that the node is in
a 'safe state'. Like, what happens if you saved half a node and you
recursively try saving that node again?  This needs a better solution.


Previous comments:

Tue, 19 Jul 2005 02:14:16 +0000 : jvandyk

Attachment: http://drupal.org/files/issues/node_serialized2x.patch (664 bytes)

node_save() saves a node, then calls insert or update hooks. These hooks
may run functions that, in turn, call node_save().

This all works fine except that if a revision is being created, the
revision gets serialized twice -- once on the original node_save() call
and once on the callback's node_save() call.

The attached two-line patch simply inserts a boolean variable called
rev_serialized to keep track of whether the serialization has already
been done.

I'd like to see cvs and 4.6 patched.

This patch is necessary so I can release actions and workflow for 4.6.

Background: http://drupal.org/node/24326


Tue, 19 Jul 2005 18:36:23 +0000 : Dries

I don't like this patch.  It's a hairy fix.  There must be a better way
to avoid recursion in first place.


Thu, 18 Aug 2005 01:26:21 +0000 : moshe weitzman

gosh, this a two line change with zero performanhce impact. seems like a
useful and minimally invasive fix for our current dilemma. we'll remove
it once a better fix comes around. is a shame to hold back
workflow/actions for such a minor issue. any others want to comment or
suggest a different solution?/


Thu, 18 Aug 2005 04:57:11 +0000 : chx

If there would be a little comment why this is needed, then I'd +1 it


Thu, 18 Aug 2005 11:59:14 +0000 : killes at www.drop.org

Please commit. In case revisions shouldn't make it (for performance
reasons ;) we need it fixed for 4.7.


Thu, 18 Aug 2005 14:22:15 +0000 : moshe weitzman

The first user of this will be workflow+actions modules. They listen on
nodeapi's insert/update operations. When a node is seen there,
arbitrary actions may fire according to the admin's wishes. So for
example, a node may be promoted or may become sticky. To do that
properly, the actions should call node_save() again with
$node->promote=1. Thus, we are recursively calling node_save() which
leads to a double serialize problem. This 2 line fixes it.

There may be more nodes which benefit from this.


Thu, 18 Aug 2005 14:23:24 +0000 : moshe weitzman

I meant to say - "there may be more *modules* which benefit from this."

More information about the drupal-devel mailing list