Not using a tmp variable causes PageTable add() to fail

[tuomassalo]

Short description of the issue

Behaviour of $page->pagetable_field->add() depends on whether a temporary variable is used:

if(isset($_GET['tmp'])) {
     // this succeeds
    $tmp = addPage('c');
    $p->ptfield->add($tmp);
} else {
    // this fails!
    $p->ptfield->add(addPage('c'));
}

Expected behavior

See test script below: ProcessWire would behave identically with or without ?tmp query string variable.

Actual behavior

With ?tmp, the script correctly returns 1. Without it, the add() fails and the script returns 0.

Optional: Suggestion for a possible fix

I have no idea. I'm not a PHP expert but I'm surprised this is even possible, without some very specific introspection hacks. Maybe the problem is related to the lifespan of objects, but I failed to create a minimal reproducible example without using ProcessWire.

Steps to reproduce the issue

Save this script to /test.php and run it with and without ?tmp.

<?php

use ProcessWire\Page;

require('index.php');
header('Content-Type: text/plain');


////// Install fixture templates, field and pages.
////// First, clean up previous run (if any).
$pages = $wire->wire('pages');
$templates = $wire->wire('templates');
$fields = $wire->wire('fields');

// Remove pages with template 'p' or 'c'.
$pagesToRemove = $pages->find("template=p|c");
foreach ($pagesToRemove as $page) $pages->delete($page, true);

// Remove templates 'p' and 'c'.
$pTemplate = $templates->get('p');
if ($pTemplate) $templates->delete($pTemplate);

$cTemplate = $templates->get('c');
if ($cTemplate) $templates->delete($cTemplate);

// Remove fields 'ptfield' if it exists.
$field = $fields->get('ptfield');
if ($field) $fields->delete($field);

// Create templates and fields.

$pTemplate = $templates->add('p');
$pTemplate->save();

$cTemplate = $templates->add('c');
$cTemplate->save();

$field = $fields->makeItem();
$field->name = 'ptfield';
$field->label = 'ptfield';
$field->type = 'PageTable';
$field->template_id = [$cTemplate->id];
$field->parent_id = 1;
$field->inputfield = 'InputfieldPageTable';
$field->save();
$pTemplate->fields->add($field);
$pTemplate->save();


function addPage(string $template)
{
    global $wire;
    $page = new Page();
    $page->template = $template;
    $page->parent = '/';

    $page->save();

    // NB: removing this line will make the test work
    $page = $wire->wire('pages')->get($page->id);

    return $page;
}

$p = addPage('p');

if(isset($_GET['tmp'])) {
     // this succeeds
    $tmp = addPage('c');
    $p->ptfield->add($tmp);
} else {
    // this fails!
    $p->ptfield->add(addPage('c'));
}
    
$p->save();

$p = $wire->wire('pages')->get($p->id);

echo $p->ptfield->count;

Setup/Environment

• ProcessWire version: latest dev
• (Optional) PHP version: 8.1 (I suspect this might be relevant!)
• (Optional) MySQL version: Mariadb 10.6

Additional observation: if I add more lines to the else block, like this:

// ...
} else {
    $p->ptfield->add(addPage('c')); // this fails!
    $p->ptfield->add(addPage('c')); // this fails!
    $p->ptfield->add(addPage('c')); // this fails!
}
// ...

... the first one fails, but the others succeed. (I confirmed this by adding a title parameter to addPage.)

[ryancramerdesign]

@tuomassalo That's a strange behavior, but from our standpoint there's no difference between add(addPage)) and add($tmp), so we might be looking at an odd PHP behavior or bug. If the add() method was forcing a pass-by-reference then we'd want to use a variable rather than a return value from another function, but that method does not use a reference argument. So I don't think that part is the source of anything on the PW side, since the two are equivalent from our interface to PHP. But we should look at the parts we do have sway over. And I am particularly curious about the line below though and how it changes the behavior. What is the purpose of it since you already have the $page?

// NB: removing this line will make the test work
$page = $wire->wire('pages')->get($page->id);

For another experiment, try changing your page save call to this:

wire()->pages->save($page, [ 'uncacheAll' => false ]); 

[tuomassalo]

I agree, it looks like a very odd PHP (compiler?) bug. Or maybe some intended PHP corner case peculiarity, since I've confirmed this bug on a wide range of PHP versions: 7.4, 8.1 and 8.4.2.