Fix deadlock when calling ActorSelection.ResolveOne from GUI context #2550

0x53A · 2017-03-07T21:10:28Z

The first test passes, the second test blocks indefinitely (even though I passed a timeout!).

This is probably an issue with a missing .ConfigureAwait(false).

SynchronizationContextHelper was copied from here: http://stackoverflow.com/questions/14087257/how-to-add-synchronization-context-to-async-test-method

0x53A · 2017-03-07T21:41:10Z

Just adding this one ConfigureAwait fixes this specific case, but I went through and added it to the whole Akka lib (excluding a few contrib and tests).

IMO this should be everywhere, and adding an await without ConfigureAwait should fail the build ...

0x53A · 2017-03-07T22:54:39Z

The test failed in CI (after my fix) with

So i guess I would need to reimplement the scheduler instead of reusing the WPF bits ...

0x53A · 2017-03-07T23:26:53Z

Nice ...

0x53A · 2017-03-07T23:28:10Z

Eh, I have set the test to skipped ... should hopefully become green now

0x53A · 2017-03-08T10:24:21Z

@Aaronontheweb I rebased it on dev, and the current failure is

which also fails in my other PR.

I assume the CI is just slightly flaky?

Danthar · 2017-03-08T10:33:16Z

Not in this case. The Akka.Tests.Performance.Actor.Pattern.AskSpec+AskThroughput test times out. Maybe a deadlock due to your latest changes ?

alexvaluyskiy · 2017-03-08T11:08:02Z

@Danthar No, we have problems with Perf tests in all pull requests

Danthar · 2017-03-08T11:26:02Z

@alexvaluyskiy But nbench tests should be reported normally in TC right? In this case it times out on invoking the nbench runner itself.
Considering other tests run fine, this leads me to believe there is something going on with the spec its running. ?

0x53A · 2017-03-08T11:33:27Z

The failing part seems to be Akka.Tests.Performance.Actor.Pattern.AskSpec+AskThroughput (http://petabridge-ci.cloudapp.net/viewLog.html?buildId=22768&buildTypeId=AkkaNet_AkkaNetWindowsPerformanceTests&tab=buildLog#_state=15933&focus=15933), which does sound suspicious. I will try to test it locally, but can only do that in the evening.

0x53A · 2017-03-08T22:25:55Z

so the good news is I was able to reproduce the failure locally:

the bad news is I was able to reproduce the failure locally :\

I have executed the failing command

C:\Users\lr\Source\Repos\akka.net\src\packages\NBench.Runner\lib\net45\NBench.Runner.exe "C:\Users\lr\Source\Repos\akka.net\src\core\Akka.Tests.Performance\bin\Release\Akka.Tests.Performance.dll" "output-directory="C:\Users\lr\Source\Repos\akka.net\PerfResults"" "concurrent="true"" "trace="true"" "teamcity="false""

twice, once for 3ae56ba (base) and once for 47b4164 (modified).

The first, aborted run (47b4164 ) took 25 minutes, exactly the timeout from build.fsx. It got until Akka.Tests.Performance.Actor.Pattern.AskSpec+AskThroughput before it was canceled.

The second run for (47b4164) finished between 9:11:42 PM and 9:37:36 PM, so it took about 26 minutes.

The third run (3ae56ba) finished between 9:43:19 PM and 10:09:06 PM, so took about 26 minutes and would also have been canceled due to the timeout.

So if I didn't mess up anything related to the build, I can't see any difference between dev and my changes.

PerfResults.zip

0x53A · 2017-03-08T22:34:00Z

The passing test run from here: http://petabridge-ci.cloudapp.net/viewLog.html?buildId=22829&buildTypeId=AkkaNet_AkkaNetWindowsPerformanceTests&tab=buildLog#_state=7528&focus=14984

It ran between [21:48:53] and [22:13:20] , so took 24:27 minutes. So the timeout of 25 minutes is maybe slightly optimistic.

Edit: after increasing the timeout, mono failed again

but Akka.Tests.Performance.dll succeded in almost exactly 25 minutes.

…eOne

Aaronontheweb · 2017-03-15T01:56:37Z

src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt

@@ -3796,8 +3796,8 @@ namespace Akka.Pattern
        public Akka.Pattern.CircuitBreaker OnClose(System.Action callback) { }
        public Akka.Pattern.CircuitBreaker OnHalfOpen(System.Action callback) { }
        public Akka.Pattern.CircuitBreaker OnOpen(System.Action callback) { }
-        public async System.Threading.Tasks.Task<T> WithCircuitBreaker<T>(System.Func<System.Threading.Tasks.Task<T>> body) { }
-        public async System.Threading.Tasks.Task WithCircuitBreaker(System.Func<System.Threading.Tasks.Task> body) { }
+        public System.Threading.Tasks.Task<T> WithCircuitBreaker<T>(System.Func<System.Threading.Tasks.Task<T>> body) { }


cc @alexvaluyskiy I get that this is an API change, but would removing the async keyword here result in a break for something that was await-ing on this task? Or is this just goofery from the API printer?

async is used as a function modifier to avoid breaking existing code that use await as a variable (and some other reasons). So basically backwards compat reason on a c# feature level.
Removing or adding it does not influence upstream dependencies at all as far as i know.

thanks @Danthar

0x53A · 2017-03-15T08:27:18Z

@Aaronontheweb is it possible to compare two runs of nbench? This does touch a lot of places, so in theory i would need to check every performance spec.
Otherwise i could revert the last commit so that only the one await is changed, reducing the diff.

Thank you.

Aaronontheweb · 2017-03-16T15:04:11Z

@0x53A this looks good to me, but the do we need to add a license header / third party notice file for the Nito source code? cc @StephenCleary

0x53A · 2017-03-16T15:11:24Z

Just to be clear, the Nito source is only used in the unit test project.
I would have used the nuget, if possible.

Regardless, credit should be given where credit is due.
So I have copied both license files to the directory, and the about.txt in the directory already links to the github repositories.

StephenCleary · 2017-03-16T15:36:18Z

It's all MIT-licensed. No problems.

Aaronontheweb · 2017-03-16T15:37:39Z

thanks @0x53A - I'm satisfied and it sounds like @StephenCleary is too. Thanks for all of your hard work. I'll merge this in once CI runs. I checked the perf numbers and everything looked fine there.

0x53A changed the title ~~Demonstrate deadlock when calling ActorSelection.ResolveOne from GUI context~~ Fix deadlock when calling ActorSelection.ResolveOne from GUI context Mar 7, 2017

0x53A force-pushed the demonstrate-resolveone-deadlock branch from e96a3c8 to 43ee88c Compare March 7, 2017 21:42

0x53A mentioned this pull request Mar 7, 2017

Consider adding an Analyzer which enforces .ConfigureAwait #2551

Closed

0x53A force-pushed the demonstrate-resolveone-deadlock branch from 13db376 to 47b4164 Compare March 8, 2017 08:36

0x53A added 5 commits March 9, 2017 01:34

check in AsyncEx from master

d3757d3

fix AsyncEx so it compiles on net 4.5

7dc7c7f

add failing testcase to demonstrate deadlock in ActorSelection.Resolv…

30b90d1

…eOne

fix failing testcase: Resolve_does_not_deadlock_GuiApplication

b300fcc

add ConfigureAwait to most of core Akka lib

6678f41

0x53A force-pushed the demonstrate-resolveone-deadlock branch from 250366a to 6678f41 Compare March 9, 2017 00:36

Aaronontheweb reviewed Mar 15, 2017

View reviewed changes

Merge branch 'dev' into demonstrate-resolveone-deadlock

0399adb

0x53A added 2 commits March 16, 2017 16:33

Create AsyncEx.LICENSE

89d29a5

Create Disposables.LICENSE

f37048f

Aaronontheweb added this to the 1.2.0 milestone Mar 16, 2017

Aaronontheweb merged commit dd39812 into akkadotnet:dev Mar 16, 2017

0x53A deleted the demonstrate-resolveone-deadlock branch April 16, 2017 14:39

0x53A mentioned this pull request Apr 27, 2021

Question on ContextInsensitive tasks rspeele/TaskBuilder.fs#35

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix deadlock when calling ActorSelection.ResolveOne from GUI context #2550

Fix deadlock when calling ActorSelection.ResolveOne from GUI context #2550

0x53A commented Mar 7, 2017

0x53A commented Mar 7, 2017

0x53A commented Mar 7, 2017

0x53A commented Mar 7, 2017

0x53A commented Mar 7, 2017

0x53A commented Mar 8, 2017

Danthar commented Mar 8, 2017

alexvaluyskiy commented Mar 8, 2017

Danthar commented Mar 8, 2017

0x53A commented Mar 8, 2017

0x53A commented Mar 8, 2017 •

edited

Loading

0x53A commented Mar 8, 2017 •

edited

Loading

Aaronontheweb Mar 15, 2017

Danthar Mar 15, 2017

Aaronontheweb Mar 16, 2017

0x53A commented Mar 15, 2017

Aaronontheweb commented Mar 16, 2017

0x53A commented Mar 16, 2017 •

edited

Loading

StephenCleary commented Mar 16, 2017

Aaronontheweb commented Mar 16, 2017

Fix deadlock when calling ActorSelection.ResolveOne from GUI context #2550

Fix deadlock when calling ActorSelection.ResolveOne from GUI context #2550

Conversation

0x53A commented Mar 7, 2017

0x53A commented Mar 7, 2017

0x53A commented Mar 7, 2017

0x53A commented Mar 7, 2017

0x53A commented Mar 7, 2017

0x53A commented Mar 8, 2017

Danthar commented Mar 8, 2017

alexvaluyskiy commented Mar 8, 2017

Danthar commented Mar 8, 2017

0x53A commented Mar 8, 2017

0x53A commented Mar 8, 2017 • edited Loading

0x53A commented Mar 8, 2017 • edited Loading

Aaronontheweb Mar 15, 2017

Choose a reason for hiding this comment

Danthar Mar 15, 2017

Choose a reason for hiding this comment

Aaronontheweb Mar 16, 2017

Choose a reason for hiding this comment

0x53A commented Mar 15, 2017

Aaronontheweb commented Mar 16, 2017

0x53A commented Mar 16, 2017 • edited Loading

StephenCleary commented Mar 16, 2017

Aaronontheweb commented Mar 16, 2017

0x53A commented Mar 8, 2017 •

edited

Loading

0x53A commented Mar 8, 2017 •

edited

Loading

0x53A commented Mar 16, 2017 •

edited

Loading