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

Shift multi-callback management from OperationalDeviceProxy to CASESessionManager #18583

Closed
mrjerryjohns opened this issue May 18, 2022 · 4 comments

Comments

@mrjerryjohns
Copy link
Contributor

Problem

Comments like this one reveal that clean-up of OperationalDeviceProxy instances on failure is just fraught with peril. Currently, callers of FindOrEstablishSession are responsible for releasing OperationalDeviceProxy instances on error. This is really error-prone. The ability for OperationalDeviceProxy to support multiple registered callbacks on the same instance complicates this story.

Proposal

Extending the idea in that comment, up-level the multi-callback management scheme in OperationalDeviceProxy to CASESessionManager. This simplifies OperationalDeviceProxy down to just managing a single callback, while permitting CASESessionManager to now take-over the responsibility of free'ing up the proxy instance on error, while still retaining the multi-callback capability of the high-level FindOrEstablishSession API.

@mrjerryjohns
Copy link
Contributor Author

FYI @msandstedt @bzbarsky-apple

@bzbarsky-apple
Copy link
Contributor

So the CASESessionManager would need to keep track of which callbacks go with which proxy, right? But yes, we could do that...

@msandstedt
Copy link
Contributor

We use the SecureSessionTable and Sessions directly, so aren't exposed to any of this. I defer to others on how OperationalDeviceProxy and CASESessionManager should work together.

@mrjerryjohns
Copy link
Contributor Author

Dupe of #19259

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants