-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Remove public Shutdown method on WriteHandler. #8713
Remove public Shutdown method on WriteHandler. #8713
Conversation
We don't ever have long-lived WriteHandler instances, so don't need external shutdown on them. Fixes project-chip#8651
I don't understand this. Write operations take more than zero time (right?), which is why we need the interaction model to asynchronously track their state in the first place. So they will be in-flight for some finite amount of time. How then do these substantively differ from any other type of interaction? |
Yes...
On the sender side. On the receiver side, the If we add async behavior under
Read and command interactions currently have the ability for the responding end (CommandHandler/ReadHandler) to do async work before responding. Write interactions do not currently have such an ability: they synchronously respond to the write with a status response. Again, this is not a feature of the spec, but just of our code. And if we change this we will want to change how the lifetime of |
OK, this answers my other question about whether writes are sync. Yes they are, and this approach relies on that. Also agree that we ought to just being doing this on the stack. |
We don't ever have long-lived WriteHandler instances, so don't need external shutdown on them. Fixes project-chip#8651
We don't ever have long-lived WriteHandler instances, so don't need
external shutdown on them.
Fixes #8651
Problem
Exposed method that can be misused.
Change overview
Stop exposing it.
Testing
Code compiles; there were no consumers of this method outside this class.