-
Notifications
You must be signed in to change notification settings - Fork 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
[Proposal] Forcing ConfigureAwait(false) on assembly level #2542
Comments
@ashmind A quick question, does the following (from #114 comment) needs static async Task<TResult> UsingAsync<T, TResult>(this T disposable, Func<T, Task<TResult>> func)
where T : IAsyncDisposable
{
try { return await func(disposable); }
finally { await disposable.DisposeAsync(); }
} assuming that it is in a library. |
@alrz I don't see why not, but I can't say I understand all the edge cases perfectly. |
Take this sample code:
You'll find that the output is this:
If a task is already completed, then the execution continues on the same thread (that might be a thread with a synchronization context). If you don't use So, to be effective, if you use |
I'd say that this is a CoreFX issue. |
@HaloFour But can it be implemented in CoreFX without option B support by compiler? |
Considering how loose the awaiter spec allows for resolving the |
+1 It's indeed a pain to have |
I'd love to see a
We've had ...or we can make |
Just want to confirm that this is a major pain for library authors. Installing analyzers into hundreds of projects and then having to set them all to Error severity just to ensure we add this ugly ConfigureAwait call is pretty de-motivating. But another alternative could be to introduce another await-style keyword as syntactic sugar for await with ConfigureAwait ( false ). Not sure what a good name would be though. |
How do you guys think of an approach with Task-like (Generalized Async Return Types): This requires no IL-weaving or no new compiler feature. |
@ufcpp pretty nice solution. Why they didnt added something like that to the TPL? |
I too find the current default maddening. What percentage does sync-requiring GUI code represent of all C# code being written today, anyway? Sigh. |
That's actually possible. We could add So that would be C Enter. |
That'd still produce hude code redundancy. Looking forward to |
Proposals for BCL additions can be made on the CoreFX repository as any such helper method won't require language changes. You'd have to describe exactly what you expect it to do. |
That's a proposal for an attribute, not a helper method. The failing of trying to manage that via an attribute is that it's far from obvious how the behavior is applied and when. A helper method that is invoked imperatively would not have that same issue. |
It would be great to have this - my code is polluted with lot of ConfigureAwait. Pleas do something with this – it should not be a big thing. |
Or maybe we can add a function-level attribute? For example:
|
It should be for method, class and assembly level ;) |
@ashmind What about: Option CIncorporate
As for per solution/project configuration means new .editorconfig options might be introduced:
|
@wachulski This is very practical, but I feel we should have more ambition and aim for cleaner code. |
Over the past year, vs-threading has reversed my view on this issue. I am very much looking forward to no longer needing to specify |
@sharwell Can you enlighten us on how vs-threading resolves this (it seems to be focused on applications, and i am doing libraries that can be used in any context)? |
@petertiedemann A good starting place is this recent document: The basic idea is a library that avoids the use of |
Almost 3 years since the proposal, |
If there's a working solution... why not use it? :) Is there actually something unreliable here? |
@CyrusNajmabadi I have yet to see an IL rewriter that correctly preserves debugging information. Also EnC won't work if you IL rewrite compiler outputs. I generally do not recommend using IL rewriters. |
It's seems like a particularly onerous restriction to eliminate this entire class of tools. It basically means that all of that functionality must always be built into the compiler. That seems unfortunate. |
Generators would address that. |
Both sound reasonable to me.
As i said above, this doesn't rise to the level that an official way is necessary. I don't view the problem as significant enough to warrant anything beyond the current story. If we had a really good solution here, i'd consider it. But we're in teh state of:
Given both of those. My position is to keep the status quo and leave this to the tooling space. |
Just deadlock because of the forgotten ConfigureAwait (false) somewhere in the depths of tons of method calls ) |
I never forget CA. That's what that analyzer is for. I don't need to remember/forget anything. The tools take care of this for me. That's why i view this better as a tooling problem
We decided that it's much saner and meets the majority case to come back to the callign context. My own experience here tells me that this is appropriate and desirable :) |
Deadlock is not the fault of a missing ConfigureAwait(false). Sometimes ConfigureAwait(true) is actually needed, and then what? It's the fault of code that decided to block the UI thread until something changes. The UI thread should never be blocked. |
Just adding my $0.02, the solution with I'd prefer the solution without an explicit way to restore the original context in order to not endorse "an unstructured/unscoped way of hopping between contexts" mentioned here: the switch to the thread pool should typically happen once at the beginning of a public method. |
How does the analyzer distinguish a forgotten ConfigureAwait from "it should be by default"? The problem is not "who write ConfigureAwait" . The problem is the need to write a bunch of boilerplate code at all, which worsens readability
If this is the UI context, then there is nothing to talk about. Non-UI context should not assume that he is "alone in the universe" |
It sounds very much like the problem is "who write ConfigureAwait". Someone needs to write it, because there are two options as to what to do and the intent must be stated. When the decision is whether to make developers write it 90% of the time or 10% of the time the decision is obvious and clear. Shrinking that down to a single character embedded in the
It's your decision to take that burden onto yourself. If those libraries are a part of a solution for an application it's not very likely that the library code will be called from under multiple contexts. It's also not particularly likely that it actually matters whether or not that library code executes within that context. If that context needs to marshal information but not jump threads then it matters even less. This discussion is going in circles and there clearly isn't an answer that will satisfy everyone. Short of proposing actual changes it doesn't seem there is really anywhere for this to go. |
Does removing ConfigureAwait not improve readability? Seriously? That's why there is a default ConfigureAwait(true), and not "force everyone to specify ConfigureAwait explicitly" because it improves readability.
I can write anything. I can even use Fody.ConfigureAwait . But I can only do this in pet projects.
It's just common sense not to predict the future. There should always be a clear contract with no side effects. Long-lived projects are regularly refactorings and any code can be moved in the future to a separate library(module) and reused in a context unknown at the time of code creation. Also ConfigureAwait (true) is a little but useless overhead |
No.
That would be because it is the majority use case. There's no need to require the developer to declare their intent to perform the default behavior. This has nothing to do with readability, it has to do with the pit of success.
That doesn't sound like a technical problem.
The question would be how realistically that situation will occur for any given project, and it's likely not very high. Using a tool to add in |
The only language that solved this problem of contexts in a sensible way is Kotlin. F# doesn't have this problem because it has its own Async. This is how F# does it . 95% of the time you don't want to bother with the Thread code runs, so ConfigureAwait default should be changed to We really need a |
There's a reliable way. Have two different Tasks, we already have ValueTask now. |
I suggest to introduce a new Keyword in the language ==> await2. That keywork would set And at the project level, an option ==> ForceAwait2 |
|
I would also certainly love to see something akin to Library authors could apply that and those of us who would actually prefer to be explicit about needed the continuation to run on the same context/tread where it is needed, could just apply that to all that we do. I am very curious about some of the all the defense made for the default chosen behavior here, to me it's one of the biggest design flaws of the TPL, and I certainly don't buy the "most people would be best off with the current default", not then, not now...
Hopefully though, most of those 80%-90% was working on something bigger than just a small little "All code in the GUI" application, and if so, then suddenly they where actually developing internal libraries. Even though a library is not shared in the world, and even if it's never shared in your organisation, doesn't mean you shouldn't treat it well. And in terms of old patterns, why would I not want my BusinessLogic or DataAccess code to execute on the first ready and available thread? Why would I at all want to lock my application to only ever execute on the GUI thread in any other places than where it was absolutely needed? - That just doesn't make any sense to me. We jumped hoops before to offload work to other threads than the GUI, utilizing the CPU's cores as best we could. So it's not about what type of developer there are most of, it's about what type of code there is most of to me, in the context of a windows forms application, if your code does not modify any form controls, draw on any surfaces etc. It's NOT UI code, and therefore should not require to execute on your UI Context, instead it should align to the recommendation for libraries, hence applying Take this small, simple, dumb - but illustrative example.
In that example, why would I not want to add that ".ConfigureAwait(false)"?, what would the need be? I would much rather have had to do:
Because then it's explicit to me. This is the piece of code that has a special requirement, not the DoHardWork method, The DoHardWork method doesn't care if it's executed on the GUI thread or not:
Therefore, DoHardWork should not need to add anything special like
Hence... it should be here I mark that I have a special requirement, namely that i require this piece of code to execute on the GUI Thread. That also means that it's no longer "MAGIC", And in clearly signals that there is a special case/requirement here.
The standing recommendation from microsoft on this matter is that library authors should probably always use Besides, any library you use provides you with a "contract" (the contract is often implicit though, so you have to discover it), if you don't uphold that contract and it means your application crashes, well then that is on you. So if a library applies Now... If we talk about flipping the "default" behavior for all Tasks tomorrow, then obviously it would be a massive breaking change that will break applications all over the world. There is no doubt about that, that is why I think that introducing a Where as solutions like The important thing is that whatever is done to ease this, is something that only affects code "in your control". Sorry if this became a bit "ranty" . |
@jeme You could take this argument even further: Should there be a These things are super dangerous. They make code non-local, non-composable and the bugs are nasty. But they are so convenient... I'm really undecided on this. I sometimes think that these contexts optimize for novice programmers and demo-ware in which the goal is to make things "just work". The more scale the codebase has the more it requires preciseness and modularity. Creating a |
An interesting approach would be to use a different version of the TPL library that always do that. There could be two versions of the same library sharing the same contract and you could choose between UI/Library by simply adding the correct reference to your project.
Certainly, that totally was a design flaw. But asking for breaking changes in working code is a obvious no go. So that's why I ask for another new incompatible and potentially different version of the TPL that does the correct thing.
Seconded, explicit is always better.
Perhaps one could create a Roslyn analyzer/rewriter. On the library side, It would to detect all uses of
I bet if some library is fooling with this, it would be better having its own Task library and Task Pool than using the shared one, in this case my solution would not work that well .
I would find it amusing if that happened, it means the library contract is badly designed, it has literal undefined behavior and just happens to work. If you get to this point, all bets are lost, you can either live with it and don't change anything, or rewrite all uses of Task library. Changing a task library is a big task, but arguably doable, if your software has proper tests, shouldn't be too bad.
As I said, asking for breaking changes in working code is a obvious no go.
This is TERRIBLE, this will do more harm than good, don't ever think about it. Better literally to not do anything than creating Global State that configures behavior of asynchronous code.
My solution of replacing |
The perils of implicit hidden mutable state. Async code should be Pure, but that's impossible to control with C#... If only there was better research on Dependent Types, you could tag the contexts in the type and it would be a compile time error to use some functions in some contexts, you could explicit code in the type the invariants you require. For example, this code needs to never run continuations in the same Thread because its a danger. You could do that with generics, but it becomes a pain very fast without
then on calling, inside another async.
Actually you could even get rid of the async/await keyword, you know you are inside Task context. But this is for future research... (if only I had a position in the Research Division, I would love to play with those things). |
If your projects never need synchronization then you'd be fine simply never setting a You call |
IMHO, this may be implemented like nullable context, with ability to enable/disable implicit *.csproj: <ConfigureAwait>false</ConfigureAwait> *.cs: #configureawait true
public static async Task ContextDependentMethod() { ... }
#configureawait restore UPD: Ok, introducing additional context is bad idea, class/method attribute for changing assembly-level default will be better. |
Two years later, and I don't see any default tooling in VS. I have to add the VS Threading library. And if tooling knows to just blindly add it everywhere, then the compiler should be able to do the same and reduce the noise. |
It isn't designed to be blindly added everywhere, though. |
dotnet_diagnostic.CA2007.severity = warning |
With the death of WinForms and WPF it should be safe to deprecate ConfigureAwait |
Someone should tell the millions of developers using WinForms and WPF that this is happening. 😁 More seriously, those technologies are far from dead - there are a huge number of actively maintained projects and products that still deliver huge value. |
If it's a non-UI library, then we are adding it blindly everywhere. We haven't had any issues yet and if we did, it would be the exception and we would be happy to explicitly set it. Even @CyrusNajmabadi says he uses tooling to add it everywhere. |
this might interest some folks: ConfigureAwait in .NET 8 by Steve Cleary reference: dotnet/runtime#87067 |
@spottedmahn nobody wants to write more It would be nice to have an option (in csproj, or via assembly attribute) to generate assembly-level default |
@spottedmahn as @epeshk wrote, we I think most people posting here don't want more of that stuff but rather less! While And that probably also goes for the In fact, it just brings some new awfulness to the API, e.g. the following is invalid and will give you a runtime exception: (it compiles just fine! - although you may raise a warning to error to avoid that)
So I am not sure, It feels more like a step in the wrong direction to me. But I don't know, maybe there is some benefits to having the API like this over a more explicit one as the first I linked. |
Problem
It's recommended to always use
ConfigureAwait(false)
in certain kinds of libraries. While it's definitely possible to use an analyzer (e.g. ConfigureAwaitChecker.Analyzer) to catch those cases, the analyzer has to be installed separately and the resulting code is awkward and verbose.Potential solutions
Option A
Provide an assembly-level attribute that would force compiler to generate (pattern-based)
ConfigureAwait(false)
calls for eachawait
.Option B
GetAwaiter([CallerMethod] MethodBase method = null)
Task
could use this overload to look for some attribute onmethod.Assembly
and return a correspondingly preconfigured awaiter.The text was updated successfully, but these errors were encountered: