Recursion bug in D7 branch when references are circular

Updated issue summary.

#1516896: Self-reference causes Drupal Core common.inc fatal error looked like the same issue from the title, but not the details. Noted also at http://geoff.com.au/drupal-module/freelinking

Setttings on site which it appears on, via drush @wiki vget freelink -

freelinking_cache: 0
freelinking_createnode_failover: "search"
freelinking_default: "createnode"
freelinking_drupalorgnid_http_request: 1
freelinking_file_file_path: "public://"
freelinking_match_syntax: "double_bracket"
freelinking_nodetitle_failover: "create"
freelinking_nodetitle_searchcontenttype: "wiki"
freelinking_path_basepath: ""
freelinking_path_failover: "none"
freelinking_prepopulate_node_advanced: Array
(
    [taxonomy] => 0
)

freelinking_prepopulate_node_type: "wiki"
freelinking_search_failover: "error"
freelinking_wikipedia_languagecode: "en"
wikitools_hijack_freelinking: 0

From #1516896: Self-reference causes Drupal Core common.inc fatal error (which mentioned the issue appearing if a page referenced itself), I wondered if this was triggered by having "recent content" block enabled, but disabling that block did not resolve the issue.

Seems this requires the Freelinking plugin falling through to the create link, Freelinking Prepopulate enabled, and a node which links to a missing node. It may be necessary to disable or clear the cache to trigger it.

These both work, as may other combinations - basically, we need to end up at createnode + freelinking_prepopulate

freelinking_default: "nodetitle"
freelinking_nodetitle_failover: "create"

freelinking_default: "createnode"

#1624874: Port project issue text filter has a similar recursion issue for Project module.

Updated issue summary.

I suspect this may require some refactoring.

node_load() seems to always trigger the text filters to run, which means that any circular freelinking references in displayed content will kill it off. I had been experimenting with using entity_load() instead to see if I could get it to not fire the node load hooks, but there was a separate issue I needed to deal with first so I stopped battling this ... for now.

@dooug, I think that I used debug_backtrace().

EDIT: Updated issue description thusly -

To reproduce: Create a node with nid 1 and containing [[nid:1]], or title "Foo" and containing [[Foo]], etc. Alternatively, two nodes which reference each other similarly work, as should three nodes referencing each other in a circle, and so forth.

Odds are the empty array is related to freelinking_prepopulate_list_fields() which (for me, anyway) gives an empty result on first call, and an array on second call. Something amiss in the static caching there?

Looks like you're aware current-page is only one cause of this bug, but just be clear - that approach probably won't help with circular (vs self) references, eg these two cases which I believe also trigger the same issue -

Title   Body
Node A  This is a node which relates to [[Node B]]
Node B  This is a node which relates to [[Node A]]

Title   Body
Node A  [[Node B]]
Node B  [[Node C]]
Node C  [[Node A]]

Thanks, but I think this isn't the right approach for a couple of reasons.

+++ b/plugins/freelinking_nodetitle.inc
@@ -18,24 +18,32 @@ $freelinking['nodetitle'] = array(
+  // get likely path title
+  $trimmed_path_title = trim($target['target']);
+  module_load_include('inc', 'pathauto', 'pathauto');
+  $link_path_title = pathauto_cleanstring($trimmed_path_title);
+
+  $source_path = drupal_lookup_path("source", variable_get('freelinking_path_basepath') . '/' . $link_path_title);
 

This looks wrong to me - both because it implicitly depends on pathauto, and because it replicates what url() should do. If you use url(), you should get the correct pathauto result anyway.

Maybe you found that this worked because url() triggered a node_load() and circumventing it didn't?

@baldwinlouie - this looks pretty good! Nice.

I managed to get one error out of it while saving a node with an intentionally broken nodetitle reference - Notice: Trying to get property of non-object in l() (line 2357 of drupal-7.16/includes/common.inc).

Unsure if that's a Freelinking issue or related to your patch, though. While the site I'm testing on is bare D7.16 with only FL and submodules enabled, I did create it a few months back so I'm not 100% of its purity. The issue I'm seeing is that $options['language'] = 'url' when passed to l() - l() expects a language object there instead of a language code.

Here's your fix above as a patch, anyway.

Setting "Needs review". Reviewer, please verify that you do not see a 'property of non-object' message if creating a broken nodetitle reference (eg [[nodetitle:ThisNodeDoesNotExist]] / [[ThisNodeDoesNotExist]]).

Suspect the test for $plugin == 'nodecreate' means that this recursion protection will not apply if the module has been configured to use different default / fallback plugins. Need to test this also.

@pmackay - grab 7.x-3.x and apply the patch above to it, then see if the issue persists?

If so, it would be helpful for you to debug - identify the block of code where you're hitting the memory limit, and add some debugging code to see how it's happening (and hopefully how to prevent it).

@deeporange1, you said #22 fixed it for you, but your changes hid the patch from #22 and left patch from #8 showing - was that intentional?

I've un-hid #22 and left #8 showing for now - please hide #8 if that was what you meant to do.