Skip to content

Conversation

@fpondepeyre
Copy link

Q A
Branch? 8.1
Bug fix? no
New feature? yes
Deprecations? no
Issues N/A
License MIT

The middleware DoctrinePingConnectionMiddleware now pings every registered EntityManager when no specific manager name is configured, instead of only the default one. Each closed manager is
reset using its own name so multi-connection setups don’t leave zombie connections. Added tests covering multi-manager pinging and targeted reset (not run locally because phpunit dependencies
couldn’t be fetched without network access).

@nicolas-grekas
Copy link
Member

But why? I mean: if there's no name configured, the default entity manager is used. So why ping should care about any others if it doesn't use them?

@fpondepeyre
Copy link
Author

@nicolas-grekas Thanks for taking a look at the MR and for the question!

Even when no manager name is configured, the middleware runs before the handler and doesn’t actually know which EntityManager the handler will use. In multi‑EM apps, handlers can inject a non‑default EM (e.g. reporting/archive). If only the default is pinged, those other connections can still be stale and you can hit “connection gone away” even though the middleware ran.

Pinging all managers in the “no name configured” case makes the default behavior safer for multi‑EM setups, while preserving the existing behavior for people who explicitly configure a single manager (they still only ping that one).

That said, if you prefer “default‑only unless explicitly configured”, it’s a reasonable position and I understand if the change is rejected on that basis.

@nicolas-grekas
Copy link
Member

Thanks for your answer. Now I'm wondering if we could ping only EM that do have an open connection? Won't the current code open new connections at the moment?

@fpondepeyre
Copy link
Author

fpondepeyre commented Dec 23, 2025

Good catch, you’re right: without a guard we’d open connections when pinging. I’ve updated the middleware to skip pings unless the connection is already open (isConnected()), so we don’t create new connections in the ‘no name configured’ case (or when the named EM is still lazy). Tests cover this change.

@nicolas-grekas
Copy link
Member

Thank you @fpondepeyre.

@nicolas-grekas nicolas-grekas merged commit 5acb383 into symfony:8.1 Dec 23, 2025
9 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants