-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Conversation
This would imply we have a task that's not CAF'd somewhere? |
CAF'd? |
I was trying to not have to type ConfigureAwait(false) ;) |
Yes, probably. How do you track those down? It would be interesting if we found it and it fixed the problem. |
I think it might be time to look at #1920 again... |
There's this: https://github.com/aelij/ConfigureAwaitChecker But I'm not sure that can be run as a solution wide (or project wide) analysis. |
A few months back I ran the ConfigureAwaitChecker on the project. The only thing that stood out to me was #2471 |
We merging this bad boy? @NancyFx/most-valued-minions |
@jchannon if we think that there is a stray |
This might be worth setting up https://github.com/DotNetAnalyzers/AsyncUsageAnalyzers One of the analyzers in there is for ConfigureAwait() |
Go go go :) On Friday, 12 August 2016, Brian Surowiec [email protected] wrote:
|
So I added this to Nancy's project.json and ran
|
@jchannon wouldn't that just let the compiler know it's a buildtime dependency? Wouldn't you have to tell something (Roslyn?) to use it to analyze the code ? |
No idea I can't find any info on how to use these analysers on Google. I On Saturday, 13 August 2016, Andreas Håkansson [email protected]
|
It looks like there's no tooling to setup/configure code analysis when you use |
So are we confident after @xt0rted PRs etc that there is no CAF issue? |
👀 |
@khellang could we update this please.. unsure if we should be merging or closing it? Thanks ❤️ |
Update? I think this should be merged ASAP. |
Done |
@thecodejunkie @khellang I believe this pull request was overwritten during the merge of #2502 specifically by commit 6e46bc4 . Should I make a PR for the change that was intended by this pull request to prevent the deadlock? I discovered this while encountering a similar issue described by the OP in #2533 and wasn't sure why I was still experiencing it, but it looks like this PR was overwritten. |
Hmm. Yeah, that's really bad. No clue how that got past a code review 😕 |
😢 @davidallyoung mind re-sending the fix? |
Sure, I should be able to get it in this evening. @thecodejunkie |
Prerequisites
Description
The call to
Task.Wait()
causes a deadlock on many concurrent requests. This changes the method to do a synchronous copy instead.Fixes #2533