Out of memory error when used with views, metatag and memcache modules

Thanks Eugene, this patch seems to fix the issue in my situation : D9.3, Search API 1.22, Metatag 1.19, Redis 1.5

#2 works (together with the patch in metatag, on solr)

This patch resolves the issue in our use case with memcache as well however it also appears to break Facets integration. I wasn't able to pinpoint exactly how with the time I had to look at this but it seems that this change somehow breaks the interactions between \Drupal\facets\Plugin\facets\facet_source\SearchApiDisplay and \Drupal\search_api\Utility\QueryHelper. For some reason the QueryHelper->getResults() does not work when the view has been cached.

My configuration is an AJAX-enabled Search API View with facets configured in a separate region. Without this patch (and with Metatags 8.x-1.16) changing a facet will update the view and the facets and then changing the same facet back to its previously setting will do the same (i.e., it works correctly). With this patch, changing a facet will update the view and facets and then changing the same facet back will cause all the facets to return empty (because they somehow lose access to the view results).

It's worth noting that Facets seems to have a pretty messy issue queue with respect to Search API interactions and Views integration generally (see #3075378: [META] Overview of Facets + Ajax issues) but there may be other places where this is used as well. I'm going to return this to NW for now.

I believe that facets doesn't support caching views that use facets and will automatically disable the cache on any view display that gets a facet assigned to it. You may be describing the reason why facets doesn't support cached views rather than an issue with this patch.

@MrDaleSmith OK I believe you are correct. My view is using Search API (tags-based) caching and it appears that is still being worked on in #2939710: Add support for "Search API (tags based)" caching in Views and the facets README says --

If you're using Search API views, make sure to disable views cache when using facets for that view.

Putting this back to NR and I will give it another test soon.

Coming back around to test this again -- for my use case if caching is disabled I no longer experience an issue with the Search API view and latest Metatag (8.x-1.19). I believe the issue I was experiencing was fixed by #3255547: Metatag Views breaks Views previews and some Views pages. My other use related to #3245876: Out of memory error with metatag_view submodule and Search API view also seems to be "fixed" by disabling cache on a Search API view with Facets integration so I'm not sure exactly what this patch is addressing anymore (: I'll leave it to someone else to move it forward.

@wells: That's great to hear! I wonder if something could be added to Search API or maybe Facets to give a warning to disable caching when using this combination, due to the problems introduced?

Possibly. Although I think its worth noting that for my use case I'm treating the recommendation (to disable cache) from the Facets module as the solution. I'm not sure if this applies to others who may be experiencing some issue without Facets in the mix (@Renrhaf? @kriboogh?). It does still seem that #3245876: Out of memory error with metatag_view submodule and Search API view is valid in some way as the Search API tag cache handler and Metatags cache wrapper are not playing nice together and this patch may be the solution. I'm just unclear at this point what the reproducible use case is to test it further.

@DamienMcKenna Facets already has code in it that disables the caching on any SAPI view that gets a facet assigned to it. It also has a note about it in its ReadMe file, but I admit that's not as visible as it could be :)

I'm also using the latest version of Metatag (which includes https://www.drupal.org/project/metatag/issues/3255547) and have metatag views enabled. I can't recreate the error either on a Search API view that doesn't use facets and has caching enabled. I have the latest version of Search API installed 8.x-1.23 so it looks to me like either the cause of the bug is misidentified, the step[s to recreate are wrong or the bug has been fixed in the latest releases of either Metatags or Search API

I'll leave it as Needs Review in case anyone else can recreate and test.

Just to let you know our story with this issue:

We use Metatag, Search API and Memcache/Redis. With the recent core security update, our Metatag version was updated from 1.16 to 1.19, because both Core Recommended and Metatag are part of our installation profile, which we update when we need to apply security updates. Search API wasn't updated, as it is not part of our installation profile, Memcache/Redis was not updated as there wasn't a new release. Shortly after applying the security update on a page using Search API, but neither Facets nor Search API Solr, we experienced a lot of segmentation faults and OOM kills of Apache for pages that had some kind of Search API view on it. Note that the Search API view used tag based cacheing. We didn't get any log entries or error messages. Note that we didn't find reliable steps to reproduce, the error would appear suddenly and then go away after some time until it would resurface. We finally managed to attach strace to a process that experienced this problem and found that PHP would try to render our Search API view and then as part of this try to retrieve the cached result for the view (or a row of the view) form Memcache. Memcache would return the result and then an endless loop would start were the result would again be retrieved from memcache and delivered by memcache until either the Apache parent process would seg fault or the Apache parent process would be OOM killed by the kernel.

We first tried updating Search API to the latest version (the other mentioned modules already were at the latest version), but the problem persisted. We then applied this patch and have not experienced any problems since then. We didn't yet have a chance to try the Metatag patch that was recently committed, looking forward to 8.x-1.20 being released, then we'll probably try that and see, if the problem resurfaces and whether this patch is still needed or if the patch to Metatag would have been enough to fix this.

I realize that this is probably not that helpful, but for now that is all the information I can provide. I'll try to update this as we get a better picture of what the problem actually was.

Re #11:

it looks to me like either the cause of the bug is misidentified, the step[s to recreate are wrong or the bug has been fixed in the latest releases

I don't see any steps to reproduce in the issue or did I miss something? For us it was kind of random, so I don't think it is easily reproduced. We tried very hard to come up with a way to reliably reproduce this and failed. What I can confirm is, that the issue has definitely not been fixed in the latest releases. To the contrary, it looks that it was introduced by a Metatag release after 1.16, probably 1.19. Looking forward to try 1.20 and see, if the Metatag patch would fix it.

After quite a bit of testing I've got a consistently reproducible use case (for me, anyway, curious to get verification from others) --

I have updated the repo I initially created for #3245876: Out of memory error with metatag_view submodule and Search API view to track more on this issue: https://github.com/CascadePublicMedia/metatag-3245876. I can no longer reproduce with just Metatags and Search API; memcache (or presumably redis) is also required to reproduce at this point.

Despite the fact that this is reproducible I'm not sure it gets us anywhere closer to understanding what the real issue here is. Ideas for further digging are welcome!

The repo README gives detailed instructions for reproduction and the gist of the environment is:

• Drupal 9.x, Search API, Metatags, and Memcache modules.
• Memcache configured as a default cache backend.
• Indexed search content in the database.
• Search API search view with caching set to "Search API (tag-based)".

I have not yet tested the patch from #2 in this environment.

Anyone else who is experiencing this issue using the flag & flag_search_api modules? We are seeing the issue most consistently on pages with search api-based views that involve the flag & flag_search_api modules.

Also just encountered this bug, but with Core's database cache backend. Figured I'd try and explain what happened in case it helps others.

It started after an update from D8.6 to 9.3 with all modules updated to be compatible. This was done in various steps, of course, not in one go. But we only noticed this bug after everything was updated (luckily on our test environment).

After running "index now", the search page works perfectly.
After clicking "Mark all items for reindexing" OR a cron run, all searches resulted in segfaults/timeouts/infinite loops much the same as mentioned in #12.
Our search view block has no Facets (another view does, but caching is disabled there as per Facets' SOP) so it does have caching based on Search API tags.

No matter when the first search occurs after the "Mark all...", the first search term segfaults and all new terms entered in the next ~4 minutes also segfault. Afterwards, new terms work properly again. All the terms used in that 4 minute timespan remain broken (because, as y'all found in this issue, it's because of unserialization during cache get).

Xdebugging lead us to MetatagViewsCacheWrapper, which lead us here.

Modules:

• core: 9.3.3
• search_api: 1.23
• search_api_solr: 4.2.6
• metatag: 1.19
• facets: 1.8.0

Patch in #2 fixes the issue.

I also just tested the patch in #2 in the reproducible environment from #13 and it resolves the issue.

I'm still hesitant to RTBC this patch though without an understanding of whether or not other functionality relies on the property removed by the change.

I think this patch could also use a more detailed docblock comment about why the property needs to be removed at this particular point but I'll leave someone else to change to NW (or tweak the comment) if others agree.

Confirmed #2 patch fixed the issue for us using solr and redis.

Thanks! Looking good to me. This ticket is RTBC.

Thanks a lot for reporting this problem and already providing a great patch, Eugene, and thanks to everyone else for testing and providing feedback!

To be honest, I also can’t really guaranteee that this change won’t cause any problems elsewhere, but since it seems to fix some major current problems for lots of sites, and also seems pretty harmless, I’m inclined to commit it.

I just have a few code style suggestions – please see, test and review the attached patch revision and I’ll commit it.

@drunken monkey changes in patch #19 look good to me -- documentation is helpful and code style is an improvement. Patch still applies and resolves the issue in my test environment so moving back to RTBC. Thanks!

#19 solved the issue for us!

#19 solves my problems.
thank you

+1 to RTBC

Out of curiosity, did anyone else only start experiencing this error after upgrading to PHP 8? When we were on 7.4, the error wasn't there. Makes me wonder if the magic methods validation change in PHP 8 was a trigger to the larger issue.

Also, line 75 of /src/Plugin/views/ResultRow.php, states "Implements the magic __wakeup() method to lazy-load certain properties.", but calls "__get()". Is that annotation in error?

Thanks a lot for the additional confirmations and feedback, everyone! Good to hear this works for so many and solves a real problem.
I just fixed the nitpicks mentioned in #21 and #25 and then committed.
I hope I didn’t forget anyone who contributed more than one short comment, my apologies otherwise.
Thanks again, everyone!

(Also, removing double-reference – linking from the other issue to this one is sufficient.)

Would it be possible to create a new release for this, since this affects quite a bit of (enterprise-grade) sites with the combination of modules that trigger this?

Agree a release here would be nice given the relation to several other metatag/core issues.

Here's a patch against 1.23 for any who need it like myself.

I can confirm this issue still persists even after the 1.25.0 release with a recursive __wakeup() call that ends with a memory exhaustion.

Please see if the patch in #3261473 helps.