Workaround PHP bug with fibers and __isset()

I've tested the fix and tests only with 11.3.x locally. Tests fail for me without Eric's change, and pass for me with it.

My understanding from https://github.com/php/php-src/issues/14983 and https://3v4l.org/ju5mp is that results (for now) are the same with PHP to 8.5, I just didn't have time to deal with environment issues to test locally against main with core@main / PHP 8.5 😬

Small comments but would be good to get exact steps to trigger this please.

@smustgrave I did my best to produce a "clean" steps to repro ... so far without success.

I see that the previous workaround #3565937: Workaround PHP bug with fibers and __get() (which this MR is a followup to) does not have steps to repro either, but we were able to agree on landing the fix there.

We have an 11.3 site where this patch reliably fixes the issue. We encounter the Fiber bug reliably both in local dev and hosted environments. The affected pages contain a number of embedded views which show Media documents (several tables, dozens of files but not hundreds); the pages use a number of contrib modules (eg DS + field templates, Advanced View Insert). Big Pipe is not enabled.

Eric and I should be able to debug & answer questions if that's helpful - we'd love to get this workaround fixed, as the PHP bug php/php-src#14983 underlying both #3565937: Workaround PHP bug with fibers and __get() and this issue is not resolved for any available PHP release to date.

Patch attached ("static" copy of the MR project/drupal!15791@1b6b9579), addresses the issue for our site(s) in the interim.

If I get the steps below working I will promote them to ID :)

Reproduction attempt

Configuration

• Require: composer require drupal/ds drupal/devel drupal/insert_view_adv drush/drush
• Install standard: drush site-install standard
• Enable: drush -y en ds devel devel_generate insert_view_adv media
• At admin/config/content/formats
  • Enable "Advanced insert view" filter for Full HTML input format
  • Add the menu button to CKEditor config
• Enable Field Templates at /admin/structure/ds/settings
• Add a reference field to term "Tags" on the Media "Document"
• Configure Media "Document" display "Teaser" field "Name" to use Field template "Expert" with tokens
  • wrapper has classes file file-type-[media:field_media_document:entity:extension]
  • field item has attributes href="[media:field_media_document]" type="[media:field_media_document:entity:mime]"
• Add a view which uses tokens to display media with relation to Tag term by TID
  • Have an excluded from display field which renders the Media with Display "Teaser" in this view
  • Use {{ rendered_entity }} in another field to show the Media

Content setup

• Generate 10 tag terms: drush devel-generate:terms --bundles=tags 10
• Generate 100 document media: drush devel-generate:media --bundles=document 100
• Create a Page node with x10 embeds of the view:
  

  <drupal-view data-view-id="table_of_media" data-display-id="default" data-arguments="1">-</drupal-view>
  <drupal-view data-view-id="table_of_media" data-display-id="default" data-arguments="2">-</drupal-view>

  , etc

Execution

When the bug is in effect, we can trigger it by drush cr and a page reload; applying the patch and repeating will produce the page output.

This is/was a known issue/major problem with the 11.3 on $entity->get('field')->entity, this is the same with isset(). I don't think this needs manual steps to reproduce, IMHO the scenario is clear and the fix is identical to __get().

I didn't review the changes in regards to the feedback or in general yet, so not setting to needs review/RTBC yet, but feel free to set back to needs review if you think the review was addressed. I am removing the tag.

Will leave for you all but not sure how to move an issue forward we don’t know why it triggers

Thanks @berdir and @smustgrave, appreciated. Review welcome :)

Not sure how people are to review if they don’t know how to trigger this. Which manes it hard to tell if this is the right fix and therefore don’t know if the test coverage is correct. Just my thoughts

I understand, but many of these fiber bugs we've seen in the last few months are very hard to reproduce by hand as you need very specific setup/data and it may also not be entirely stable in how it happens.

If you expand the scope in the MR up, you can see the exact same code in __get() (with the only difference being the parent call). FWIW, I think two cases isn't worth trying to generalize this, there shouldn't be third and it would make it even harder to read. As far as I'm concerned, that is sufficient in proving that this is the correct fix for now. We also hope to eventually remove this again, either in the open follow-up #3566626: [12.x] Use property hooks for EntityReferenceFieldItemList->entity or when php fixes this bug.

I haven't reviewed the tests yet, so not yet ready to RTBC, but I'll try to do that soon.