Detect infinite hooks in plugin discovery hooks and/or event subscribers

Created on 16 November 2017, over 6 years ago
Updated 27 February 2024, 4 months ago

Not sure whether this needs to be tagged as 'plugin system' or 'base system' because it affects both in a really nasty way.

When plugin managers discover their plugin definitions, they send them to an alter hook. If an alter hook then queries a different plugin manager or event firing system for info, we might run into an infinite loop if an implementation of said system queries the original system that is still building its definitions.

Example

Views' route subscriber adds the entity type manager having discovered its definitions as a hard requirement in its constructor as it asks for a storage handler. If you write a hook_entity_type_alter() implementation that needs to ask a route for data it will:

  1. Start building the routes
  2. Get to Views' route subscriber which needs the entity type definitions
  3. This causes an entity type manager discovery
  4. This causes a route discovery
  5. etc.

The above is exactly what happened with the latest patch from this issue: #2651974: field_ui_entity_operation() cannot respect route parameters because of incorrectly named routes β†’

But the problem is, this could happen with any system that needs to be built if said system fires any events or alter hooks in the process. We simply cannot control nor predict what an implementer might need in order for them to successfully respond to the event or alter a definition. As shown by Views' route subscriber having a hard requirement for the entity type manager.

When discussing this on Slack, tim.plunkett said "I generally wouldn't consult any other build-time services during other build-time code" and I totally agree with that. Yet, Views is doing the exact opposite. And I can't blame it for doing so: It needs the storage handler so it can load a view to properly define its routes with. Likewise, in the other issue, Field UI needs the field_ui_base_route's path so it can define link templates correctly.

So I'm not saying we should fix Views, because that would be treating the symptoms instead of the cause. But by not doing anything, we're slowly but steadily installing deadlocks in core on a first come, first served basis. "Want to query a route during an entity type alter? You can't because Views already has some code that will cause an infinite loop if you do."

Imagine if we start having more of the above. We'd end up with a core where we have to carefully document every discovery alteration hook or event that is fired saying: "Here are the things you can't rely on for this particular function because someone else already wrote some code that closed the door for you."

Proposed resolution

I have no clue.

The lazy route builder and regular route builder have a system that at least stops the recursion as shown in the other issue. But that does not fix the cause, it just prevents it from blowing up your server :)

We can't go and tell people they can't query any system during builds either, because that would obviously break a lot of existing implementations such as Views' route subscriber.

So what can we do?

πŸ“Œ Task
Status

Active

Version

11.0 πŸ”₯

Component
PluginΒ  β†’

Last updated about 18 hours ago

Created by

πŸ‡§πŸ‡ͺBelgium kristiaanvandeneynde Antwerp, Belgium

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡¬πŸ‡§United Kingdom catch

    Re-purposing this as a task - it'd be a DX improvement to warn people, but I also have no idea how we can do this. A lot of hooks/events are re-entrant and some contrib relies on this, although in slack @longwave suggested an assert() which might work.

Production build 0.69.0 2024