Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve interop in long-running processes #98

Open
Firehed opened this issue Mar 7, 2019 · 1 comment
Open

Improve interop in long-running processes #98

Firehed opened this issue Mar 7, 2019 · 1 comment
Milestone

Comments

@Firehed
Copy link
Owner

Firehed commented Mar 7, 2019

In runtimes like ReactPHP, PHP-PM, and Roadrunner, the same instance of a Dispatcher will tend to be reused across multiple requests. This can result in weird stateful interactions across multiple requests. While this is to some extent unavoidable (without making the Dispatcher completely stateless, and requiring a setup/teardown on every request), there should be clearly-defined boundaries to reduce pain here:

  • Any setup process should only have to be performed once, during the server's bootstrap (set endpoints, container, etc)
  • The dispatcher should fetch fresh instances of any dependencies from the container (or a custom variant) on each request, and avoid holding them in internal state whenever possible. In fact ideally, absolutely no state exists in the container beyond what happens in bootstrapping

Candidates for fixing:

  • AuthnProvider
  • AuthzProvider
  • ErrorHandler
  • ServerRequestInterface (pass to Dispatch directly as a paremeter)

Middleware is questionable - I think it's reasonable to require that users in this environment either use only stateless middleware, or at least are careful to avoid weird cross-process dependency chains.

This is very much a breaking change - the direct setters for the above would have to be removed, and depend entirely on container convention.

The good news is that a fair bit of statefulness has already been stripped out of the dispatcher in v4 since this concept was floating around during the development cycle.

In terms of performance, a traditional PHP-FPM setup should be entirely unaffected. This may lead to very slight degradation in long-running processes, but likely on the order of microseconds with the upside of being way more correct behavior and less likely to leak memory or have to use insane workarounds to avoid leaks.

Context: I was able to locally patch the dispatcher to avoid holding onto the authn/authz providers, allowing them to be freed after the request was processed. This removed a reference to doctrine, which allowed it to be freed (the authn provider appeared to have the last reference to it). Then a lot of cleanup stuff worked.

@Firehed
Copy link
Owner Author

Firehed commented Mar 8, 2019

In terms of state management, I think this is basically fixed. Docs could use additional work, and I think there's room for improving the use of ClassMapper to avoid rebuilding that on every request (though looking back on the internals, maybe not).

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

No branches or pull requests

1 participant