Add AsyncLazy<T>.SuppressRelevance() method
#1254
Conversation
This allows value factories to identify their own code blocks that won't be awaited on and therefore shouldn't trigger reentrancy detection.
There was a problem hiding this comment.
The PR looks good and would provide a way to resolve this problem for some scenarios. It might be hard to use in other situations.
What we ran into (inside CPS), is that lazy initialization may trigger another sets of lazy initializations (for example, one set of data sources would trigger its own dependencies to be initialized). During a chains of initialization to happen, one base lazy computation might spin off a task capturing the execution context (for example, link to a dataflow with data ready). The captured evaluation context could have a whole stack of AsyncLazy context. This API is actually fairly difficult to use due to multiple layers of code.
Also, evaluation context can be captured in so many ways, adding a continuation of a task, or registering a cancellationToken could do, which makes it hard to know where to suppress them for many developers.
On the other side, this does provide a way to suppress the check completely, if the developer chooses to do that. I am not sure the check or suppressing the check too broadly is a good thing or not. But the developer has at least a chance to make the call based on the situation.
|
Good points, @lifengl. If you have any ideas for how to improve on this for the scenario you described, I'm interested in further improvement in this area. |
after thinking about that, I think maybe it should provide an option to disable the check completely. Yes, when AsyncLazy has a path to depend on itself, it will hang. It could cause product to hang, but with modern debugger features, it is actually much easier to investigate the hang with a dump, than the exception from the detection logic as it today. It is actually almost impossible to understand what is going on from the exception, and also this work comes with overhead. Also, in the history, almost all issues caught by this logic in our usages are false alarm from ExecutionContext leaks (during the loading phase, when there are many lazy initial logic, it is much easier to leak it in a way very hard to figure out, and the exception doesn't help to find the source in almost all the cases). A normal JTF.RunAsync might form a loop as well, if it eventually awaits on itself, and there is no logic to detect that -- and, in reality, it almost never happens. Maybe it I happens more often in the AsyncLazy case, but I feel maybe a hang is actually better result. |
|
You make excellent points. I'm working on a way to disable it for the whole instance. |
This allows value factories to identify their own code blocks that won't be awaited on and therefore shouldn't trigger reentrancy detection.