Memory exhaustion when setting script translations on `wp-i18n`

[aduth]

A memory exhaustion occurs given the following snippet of code:

<?php
add_action( 'admin_enqueue_scripts', function() {
        wp_set_script_translations( 'wp-i18n', '' );
} );

<b>Fatal error</b>:  Allowed memory size of 268435456 bytes exhausted (tried to allocate 262144 bytes) in <b>/srv/www/editor/htdocs/wp-includes/class.wp-dependencies.php</b> on line <b>324</b><br />

Memory exhaustion occurs here:

​https://github.com/WordPress/WordPress/blob/4e3e9ca9cf893190609728848caf224034fd1ef0/wp-includes/class.wp-dependencies.php#L338

I think it stems from this line:

​https://github.com/WordPress/WordPress/blob/4e3e9ca9cf893190609728848caf224034fd1ef0/wp-includes/class.wp-scripts.php#L517

Where wp-i18n is adding itself as a dependency.

Could probably do for a simple check to ensure that the incoming handle is not wp-i18n.

[aduth]

It's a bit more complex unfortunately, since even with the patch, the same issue would occur when calling wp_set_script_translations with wp-polyfill, since wp-polyfill is itself a dependency of wp-i18n.

​https://github.com/WordPress/wordpress-develop/blob/ad0a1a2f8280bb421d36a0f1080080e7c0f7c6d8/src/wp-includes/script-loader.php#L425

Handling this then becomes even more clumsy:

<?php

if ( 'wp-i18n' !== $handle && ! in_array( 'wp-i18n', $obj->deps, true ) &&
                ( ! isset( $this->registered['wp-i18n'] ) || ! in_array( $handle, $this->registered['wp-i18n']->deps ) ) ) {
        $obj->deps[] = 'wp-i18n';
}

Since context was omitted, the issue becomes most apparent from the Gutenberg plugin in trying to re-register all of the packaged scripts. A workaround will be implemented there in the meantime.

​https://github.com/WordPress/gutenberg/pull/12559

[desrosj]

@aduth are you able to refresh the patch based on everything learned in ​GB-1559?

[JeffPaul]

Per input from @aduth in today's bug scrub and with minimal movement for this in the 5.2 release cycle, we're punting this to Future Release.

[aduth]

The last patch aligns to what was implemented with ​GB-1599. There's still room for improvement:

• The condition with 'wp-polyfill' ought to really be testing any of the dependencies of 'wp-i18n'
• Generally speaking, I'm thinking this would be better served with an improvement to the lower-level resolution of circular dependencies of scripts, so as to avoid the possibility for this to occur for _any_ script enqueue, particularly since the error is quite cryptic (@iseulde had encountered a similar problem for a different script).

To clarify, there's not a huge urgency to fix this, as best I can tell. But the snippet from the original comment is definitely not something one would expect to cause a memory exhaustion.

[VadimNicolai]

I understand that this is not a huge urgency.
But for me, as a GB contributor, it is a real pain.
Can we have a slightly higher priority on this?

[aduth]

@VadimNicolai , as ​mentioned in Slack, this should not be an issue for out-of-the-box use of either WordPress or Gutenberg. It requires specific code to be run for the memory exhaustion to occur. If you're encountering it, it's likely there is some combination of plugins or local changes which is prompting the error.

The purpose of the ticket is more in providing safeguards in core to prevent this from ever occurring. It could also be expanded to the broader concern that the script loader allows for circular dependencies to be resolved recursively (vs. disallowing registration of these dependencies or bailing out upon detection of the circular dependency).

[noisysocks]

It would be good for a proper fix here to land in 5.7 so that we don't need to ship a workaround. See GB28279.

[noisysocks]

Replying to aduth:

Memory exhaustion occurs here:

​https://github.com/WordPress/WordPress/blob/4e3e9ca9cf893190609728848caf224034fd1ef0/wp-includes/class.wp-dependencies.php#L338

I tested this and it looks like now the memory exhaustion error now happens in all_deps and not recurse_deps. This is likely because, since this ticket was opened, recurse_deps was updated to no longer be recursive. See #46469.

Maybe we can update all_deps similarly.

[noisysocks]

I did some experimentation here and 46089.diff​ is a very rough proof of concept for what I described in my comment above. It fixes the infinite recursion but needs tests, testing, and to be prettied up.

It looks as though we need the workaround in GB28279 even if this bug is fixed, though. This is because WordPress will print a script’s translations before it prints the script. This means that wp-i18n’s translations will try to call wp.i18n.setLocaleData() before wp.i18n is defined.

For this reason I am moving this ticket out of 5.7 and will create a new ticket which implements the workaround in GB28279.