node_load in menu_node_menu_link_insert is not safe

I could find any other way to avoid the node_load except for mocking up the node object and the let it be up to the hook implementers to decide if they want to load the node.
But can't recommend doing it as it will return and data empty node object if you create a menu link doing a field_attach_insert call.

Would think so yes, will test it in a plain Drupal installation tomorrow and get back to you with a way of replicating this issue

It seems that I the above assumption of how the core book module works doesn't trigger this issue.

The main point of this issue is that if you have a field, that generates a menu item (being that it calls a "menu_link_save") it will then trigger menu_node's menu_node_menu_link_insert function which will try to load a node which haven't yet been properly saved.

Reason I trigger this error, is that we have started work on a better way of implementing books. Where one of the main aspects is that the book is created as an entity, which hold a reference to the menu and to the initial node which created the book. Further more the way to tell other entities that they can be part of a book is by adding a field which allows adding it to a book. Each element added to the book then holds a reference to the book entity, what I wanted to use menu_node to was to actually being able to handle alot of the aspects of rendering structure of the book through views. So also using "menu_node_views".

But back on topic, has this supplied you with enough information to consider the above patch code?

I am not exactly sure how to deal with it, but in general I don't think it is safe to assume that we can always load the current node that is being inserted.
I do not see any issue on update, but the insert action can be harmfull.

From my perspective it seems to be smarter to let the hook implementers to decide if they consider it safe to load the node, so basically changing this to pass around the nid instead of an entire node object.

No we are not modifying it outside of node_save, in the function it calls "field_attach_$op" (line 1130 - 1132 in latest stable)

<?php
 // Save fields.
    $function = "field_attach_$op";
    $function('node', $node);
?>

So the stack trace originates from node_save, but goes through the field_attach_insert hook, and onwards to handle the individual field widget submit handlers.
In one of these we are handling data which basically adds the current node to the menu system and saves information about this node being part of a specific book entity.

Hope this makes sense - other option would be to publish the code, but would need to clear it with the original creators of the code first.

I am waiting for the guys to come home from Denver or at least resurface so i can ask them.

Will postpone it untill I talk with them.

I have pushed the code for our book module into a sandbox, notice this module is still in early development state.
Betterbook - http://drupalcode.org/sandbox/lslinnet/1497328.git

You can replicate the error by:

• Enable both betterbook and menu_node
• creating a content type with a book field.
• Create the first node of the content type (so that a book is created)
• Create a new childpage to the book.

This should now show you a node where the main body field and book field is empty.
If you go to the edit page of the newly created node it looks like the node is empty.
But if you look at it in the database or after cache have been cleared, all the fields will be there.

The above approach is still the only way to handle menu insertion from a field widget doing node insertions.