Skip to content

Reset DbSwitchEventListener with the container - #94

Open
gisostallenberg wants to merge 1 commit into
RamyHakam:v4from
gisostallenberg:reset-tenant-switch-state-v4
Open

Reset DbSwitchEventListener with the container#94
gisostallenberg wants to merge 1 commit into
RamyHakam:v4from
gisostallenberg:reset-tenant-switch-state-v4

Conversation

@gisostallenberg

Copy link
Copy Markdown

DbSwitchEventListener already implements ResetInterface, but its service definition is written out explicitly without autoconfigure(), so the ResetInterface autoconfiguration never applies and reset() is never called. TenantContext, on the other hand, is tagged kernel.reset. The two therefore desync in any long-running process:

  1. Message/request 1 switches to tenant X — the listener remembers X and TenantSwitchedEvent sets the context to X.
  2. The container is reset between messages (messenger:consume) or between requests (FrankenPHP worker mode) — the context is nulled, the listener still holds X.
  3. Message/request 2 switches to X again — the listener early-returns on currentTenantIdentifier === X and never dispatches TenantSwitchedEvent, so the context stays null while the connection is on X.

Anything asking "is a tenant active?" then reads null for the rest of that worker's life, even though the tenant connection is live and correct.

Why the listener may be reset — and should be: its state is a pure memoisation of the last switch, and reset() only drops that memory. The very next SwitchDbEvent performs the switch it would otherwise have skipped, so the only cost is repeating work that is idempotent by construction: resolve the tenant config, clear the entity manager, point the connection at the same database. Nothing that survives a container reset depends on the skipped path — and the bundle already treats these two as one unit in TenantTestTrait:: resetTenantState(), which resets the listener and the context together. Tagging the listener kernel.reset gives production the same pairing the test helper has always had.

DbSwitchEventListener already implements ResetInterface, but its service
definition is written out explicitly without autoconfigure(), so the
ResetInterface autoconfiguration never applies and reset() is never called.
TenantContext, on the other hand, is tagged kernel.reset. The two therefore
desync in any long-running process:

  1. Message/request 1 switches to tenant X — the listener remembers X and
     TenantSwitchedEvent sets the context to X.
  2. The container is reset between messages (messenger:consume) or between
     requests (FrankenPHP worker mode) — the context is nulled, the listener
     still holds X.
  3. Message/request 2 switches to X again — the listener early-returns on
     `currentTenantIdentifier === X` and never dispatches TenantSwitchedEvent,
     so the context stays null while the connection is on X.

Anything asking "is a tenant active?" then reads null for the rest of that
worker's life, even though the tenant connection is live and correct.

Why the listener may be reset — and should be: its state is a pure memoisation
of the last switch, and reset() only drops that memory. The very next
SwitchDbEvent performs the switch it would otherwise have skipped, so the only
cost is repeating work that is idempotent by construction: resolve the tenant
config, clear the entity manager, point the connection at the same database.
Nothing that survives a container reset depends on the skipped path — and the
bundle already treats these two as one unit in TenantTestTrait::
resetTenantState(), which resets the listener and the context together.
Tagging the listener kernel.reset gives production the same pairing the test
helper has always had.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant