[development] [bug] nodeapi calls terminated because of node_loads

[Bèr Kessels]

Issue status update for 
http://drupal.org/node/40886
Post a follow up: 
http://drupal.org/project/comments/add/40886

 Project:      Drupal
 Version:      4.7.0-beta1
 Component:    node system
 Category:     bug reports
 Priority:     critical
 Assigned to:  chx
 Reported by:  Bèr Kessels
 Updated by:   Bèr Kessels
 Status:       patch (ready to be committed)

@dries. I have no clue, really :p This array-memoy-space-pointer-stuff
is way over my head. CHX, mind sharing your knowledge here?


Oh, and chx, do you mind changing this to 'normal' instead of critical?
so far, in about two years, i'm the only one who ever encountered it,
and that even for a rather unpopular module.




Bèr Kessels



Previous comments:
------------------------------------------------------------------------

Mon, 12 Dec 2005 16:38:36 +0000 : Bèr Kessels

Attachment: http://drupal.org/files/issues/node_module_nodeapi_oddness.patch (873 bytes)

in clipper module I ran against a breally odd issue. When enabled, about
half the other modules' nodeapi hooks would not get called.
flevour [1] hopped on for a wild bughunt, and came with this solution:


instead of doing a 
<?php
 foreach (module_implements('nodeapi') as $name) { 
?>

 we call the module_implements('nodeapi') before. 


Here is how to reproduce:
Just add a random node_load(1) to any nodeapi insert, update or
validate, in a random module (well, not upload, because that is the
last one ran, most prolly :) ).
You wil then notice that half of the data in your node/add or /edit
screens are no longer saved, nor checked. 


This tiny patch fixes the issue. I have no clue why, but it does fix
it. :)
[1] http://www.flevour.net/




------------------------------------------------------------------------

Mon, 12 Dec 2005 16:59:44 +0000 : chx

This is sheer impossible. I'll investigate more but there is no way
module_implements' internal cache (a static variable) gets overridden,
after that cache is build it's only returned. There is no reset
facility as there is no need.




------------------------------------------------------------------------

Mon, 12 Dec 2005 17:02:00 +0000 : chx

I am babbling because of the shock.


foreach operates on a copy. it does not matter whether
module_implements is immutable. Ber, what PHP version do you run?




------------------------------------------------------------------------

Mon, 12 Dec 2005 17:38:39 +0000 : Bèr Kessels

Attachment: http://drupal.org/files/issues/node_module_nodeapi_oddness_0.patch (900 bytes)

I find it funny that you say it is impossible, while the preactice
proves that this fixes the issue. 


 "One should really learn to beleive reality, even if theory doesnt
allow that reality"


anyhow, a slighly modded patch, one that adds a comment.




------------------------------------------------------------------------

Mon, 12 Dec 2005 18:08:04 +0000 : chx

As said, I am babbling.


Second, while the patch is patch good, I told you, I fear about what
else is broken. Not what your patch brokes, but there is an unknown
cause and that cause might have broken other stuff too.




------------------------------------------------------------------------

Mon, 12 Dec 2005 18:39:36 +0000 : chx

Attachment: http://drupal.org/files/issues/php_bug.patch (312 bytes)

I chatted with Goba and we worked around the problem at its root.




------------------------------------------------------------------------

Mon, 12 Dec 2005 18:48:41 +0000 : chx

Attachment: http://drupal.org/files/issues/php_bug_0.patch (521 bytes)

Much better comments. This is super-duper hard.




------------------------------------------------------------------------

Mon, 12 Dec 2005 18:56:36 +0000 : Goba

This fixes the issue much more generally, and should be committed. (The
issue is that arrays in arrays are stored with references).


+1




------------------------------------------------------------------------

Mon, 12 Dec 2005 19:15:04 +0000 : chx

Attachment: http://drupal.org/files/issues/php_bug_1.patch (570 bytes)

Linking relevant Zend docs.




------------------------------------------------------------------------

Tue, 13 Dec 2005 15:55:43 +0000 : Dries

I'm not sure I understand the PHP comment.  What do you mean with 'the
array pointer will be off'?  Can you try to clarify that part of the
code comment?  Thanks.