Could you pull the latest master? It was changed in https://gerrit.wikimedia.org/r/#/c/244044 and then partially reverted in https://gerrit.wikimedia.org/r/#/c/249189/

Could you pull the latest master?

I think I mentioned that 1.27alpha (ab1e69b) is used which according to

It was changed in https://gerrit.wikimedia.org/r/#/c/244044

Moving before the cited change allows again to carry out move actions, so this is certainly a regression.

After a month of inactivity on this task, I just made a new attempt (1.27alpha (d545912)) after the following comment "The removal of the custom error handler for Hooks::run() is in 1.27.0-wmf.8 which should be active on all wikis now. I tested a page move on wikitech and could not recreate the error:" by bd808 [0] which most likely has the same cause.

I tried to move a page and it reports with "Parameter 3 to SMW\MediaWiki\Hooks\{closure}() expected to be a reference, value given in ...\includes\Hooks.php on line 195".

The closure is defined as function ( &$oldTitle, &$newTitle, &$user, $oldId, $newId ) and has been since the usage of MW 1.19.

[0] https://phabricator.wikimedia.org/T120218

This might be a bug in HHVM. I modified the code as follows:

		$params = [
			&$this->oldTitle,
			&$this->newTitle,
			&$user,
			$pageid,
			$redirid,
			$reason,
			$nullRevision
		];
		
		var_dump( 'LOLXX', $params );
		$dbw->onTransactionIdle( function () use ( $params, $dbw ) {
			// Keep each single hook handler atomic
			$dbw->setFlag( DBO_TRX ); // flag is automatically reset by DB layer
			var_dump( 'LOLYY, $params );
			Hooks::run( 'TitleMoveComplete', $params );
		} );

At LOLXX $params[2] is reference, but at LOLYY it is only plain value. If I immediately invoke the anonymous function, it is also a reference.

I don't know how I would convince any knowledgeable person to look into this. I believe if I were to report this to HHVM upstream, they would ask minimal test case, but I can't provide anything but MediaWiki as-is. I am running HHVM 3.12.0 btw. It would be nice if someone could test this with PHP.

In T118683#2064791, @Nikerabbit wrote:

I believe if I were to report this to HHVM upstream, they would ask minimal test case, but I can't provide anything but MediaWiki as-is. I am running HHVM 3.12.0 btw. It would be nice if someone could test this with PHP.

I think I have a repro case:

<?php
function expectsReference( &$reference ) { echo "ok\n"; }
call_user_func_array( 'expectsReference', [ [] ] );

Outputs:

Zend:

$ php5 closure.php
ok

HHVM 3.12.1:

$ hhvm closure.php
[Tue Mar  8 01:04:48 2016] [hphp] [2782:7f388c1dc100:0:000001] []
Warning: Parameter 1 to expectsReference() expected to be a reference, value given in /home/ori/closure.php on line 3
ok

Why was this closed? I encountered this issue on PHP 5.6 as reported (not HHVM) and T115781 only makes references to HHVM not PHP.

I, too, don't think this is an HHVM issue - I got a report of it happening with MW 1.27.0 and PHP 5.6.24, no HHVM in use.

The problem also occurs with MW 1.27.1 - I just tried it out myself. (And PHP 5.6.25, and no HHVM.)

How can MW 1.27.1 be stable with a broken hook?

PHP 5.5.9-1ubuntu4.14 (cli)

In T118683#2682645, @Subfader wrote:

How can MW 1.27.1 be stable with a broken hook?

To answer the obvious, AFAIS this was never triaged as a 1.27 release blocker, I've added the proper tag now. (Also removing HHVM since it's not an HHVM bug apparently).

Change 361776 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[mediawiki/core@master] [WIP] Testing TitleMoveComplete hook with &

https://gerrit.wikimedia.org/r/361776

Seems to be caused by the inherited parameter going out of scope. Here is a minimal reproduction:

global $hook;
$hook = function ( &$reference ) { echo "ok\n"; }

$wrap = function() {
    $user = [];
    $params = [ &$user ];
    $f = function () use ( $params ) {
        global $hook;
        call_user_func_array( $hook, $params );
    };
    echo "inner\n"; call_user_func( $f ); // works
    return $f;
};

$f = $wrap();
echo "outer\n"; call_user_func( $f ); // fails

Defining $f as function () use ( &$params ) does not help either.
(Tested on 5.6.99-hhvm.)

The simplest workaround I could find is to unpack/repack the parameter array:

global $hook;
$hook = function ( &$reference ) { echo "ok\n"; }

$wrap = function() {
    $user = [];
    $params = [ &$user ];
    $f = function () use ( $params ) {
        global $hook;
        list( $user ) = $params;
        call_user_func_array( $hook, [ &$user ] );
    };
    echo "inner\n"; call_user_func( $f ); // works
    return $f;
};

$f = $wrap();
echo "outer\n"; call_user_func( $f ); // works

It will break the reference but with the callback moved to post-commit it wouldn't do anything anyway...

In T118683#3385231, @Tgr wrote:

Seems to be caused by the inherited parameter going out of scope. Here is a minimal reproduction:
<snip>
(Tested on 5.6.99-hhvm.)

Based on https://3v4l.org/XbNo3 it appears this works in PHP 7, but is broken in PHP 5 and HHVM.

Here's a more minimal reproduction: https://3v4l.org/dlubp

Apparently PHP5 and HHVM convert a reference into a non-reference when no other reference to the value exists (i.e. the refcount goes to 1), much like how an existing non-reference is turned into a reference when you take a reference of it.

Another, simpler, workaround would be to just keep more than the one reference to $user live as long as $params is, e.g. with use ( $params, &$user ). Or you could do $tmp = &$params[2]; to turn it back into a reference just before the Hooks::run() call.

Also refs https://github.com/SemanticMediaWiki/SemanticGlossary/issues/40

(Un)fortunately this issue will now only affect HHVM, since PHP 7 handles this correctly, and we no longer support <=5.6.

Change 438078 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[mediawiki/core@master] Ensure $user is passed by reference in TitleMoveComplete hook on HHVM

https://gerrit.wikimedia.org/r/438078

Change 438078 merged by jenkins-bot:
[mediawiki/core@master] Ensure $user is passed by reference in TitleMoveComplete hook on HHVM

https://gerrit.wikimedia.org/r/438078

Change 438249 had a related patch set uploaded (by Krinkle; owner: Legoktm):
[mediawiki/core@REL1_31] Ensure $user is passed by reference in TitleMoveComplete hook on HHVM

https://gerrit.wikimedia.org/r/438249

Change 438251 had a related patch set uploaded (by Krinkle; owner: Legoktm):
[mediawiki/core@REL1_30] Ensure $user is passed by reference in TitleMoveComplete hook on HHVM

https://gerrit.wikimedia.org/r/438251

Change 438254 had a related patch set (by Krinkle) published:
[mediawiki/core@REL1_27] Ensure $user is passed by reference in TitleMoveComplete hook on HHVM

https://gerrit.wikimedia.org/r/438254

Change 438249 merged by jenkins-bot:
[mediawiki/core@REL1_31] Ensure $user is passed by reference in TitleMoveComplete hook on HHVM

https://gerrit.wikimedia.org/r/438249

Change 438251 merged by jenkins-bot:
[mediawiki/core@REL1_30] Ensure $user is passed by reference in TitleMoveComplete hook on HHVM

https://gerrit.wikimedia.org/r/438251

Change 438254 merged by jenkins-bot:
[mediawiki/core@REL1_27] Ensure $user is passed by reference in TitleMoveComplete hook on HHVM

https://gerrit.wikimedia.org/r/438254

Change 361776 abandoned by Legoktm:
[WIP] Testing TitleMoveComplete hook with &

https://gerrit.wikimedia.org/r/361776