# Prevent recursive calls in property request methods * Status: accepted * Deciders: dequbed * Date: 2023-01-30 See also the [Github issue] ## Context and Problem Statement Right now there is a potential issue with Callbacks that need to aquire locks; if a mechanism calls `session.need_with` and friends from within the closure of `session.need_with` a second call to `Callback::callback` will be issued before the first one completes. If e.g. a Mutex is aquired inside the first function call it will likely not be released by the second call, leading to a deadlock. We should either explicitly document Callbacks not being allowed to hold locks over a call to `provide` or prevent mechanism from calling `need_with` recursively. The latter can easily be done via type system by making `need_with` take `&mut self,` the latter is likely much harder and potentially worse from an user perspective. ## Decision Drivers * Implementations of mechanisms know very little about their uses and are supposed to be as plug-and-play as possible; issues like the above can lead to hard-to-debug interaction bugs. ## Decision Outcome Chosen option: "Prevent recursive callbacks", because the compiler can more easily enforce this rule: by taking a `&mut self` in the MechanismData property methods recursive calls are prohibited. ### Positive Consequences * A number of interaction bugs and edge-cases are prevented * Developers are informed of improper use right away by descriptive compiler messages ### Negative Consequences * A number of optimizations to forgo allocation and memory copying aren't possible since the lifetimes or provided properties does not extend outside the callback. For example it's not possible to request a value for `OverrideCBType` and request the cb data from within the response callback. ## Pros and Cons of the Options ### Prevent recursive callbacks * Good, because it prevents interaction bugs * Bad, because it prevents a number of small optimization and will force more allocations * Neutral, it's additionally always possible to drop this restriction again as going `&mut self -> &self` is not a breaking change. ### Allow recursive callbacks * Good, because it allows for some optimizations * Good, because it minimizes the limits the rsasl API forces upon mechanisms * Bad, because it increases the chance of deadlocking * Bad, because while it minimizes limits it does so by putting further assertions on callbacks that are not immediately obvious from the API and function prototypes. ## Links * [Github issue] [Github issue]: (https://github.com/dequbed/rsasl/issues/18)