Skip to content
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

(net35) System.Threading polyfill dependency incorrectly flows SynchronizationContext #12

Open
jnm2 opened this issue Jan 3, 2018 · 3 comments
Milestone

Comments

@jnm2
Copy link
Collaborator

jnm2 commented Jan 3, 2018

For reference, this demonstrates the bug: https://github.com/jnm2/AsyncBridge/compare/sync_context_flow_bug

This differs from the .NET Framework 4.5 behavior. Example of what can go wrong:

// UI thread
await Task.Run(async () =>
{
    // Thread pool thread
    DoCPUIntensiveWork();
    await DoIOBoundWorkAsync();
    // PROBLEM: the await resumes on the UI thread, not a thread pool thread!
    DoCPUIntensiveWork(); // Ends up blocking the UI thread
});

The workaround is to await DoIOBoundWorkAsync().ConfigureAwait(false). (Once v0.3.0 is out with the fix for #7!) To be clear, ConfigureAwait(false) is better code, but it should still not be necessary.

Looks like another easy fix. The problem is, the source code for TaskParallelLibrary 1.0.2856 is not on the internet as far as I can tell. The nupkg and binaries are first committed to this repo in f97269a and had been downloaded from https://www.nuget.org/packages/rx-core/1.0.2856. I've scoured https://rx.codeplex.com/ and https://github.com/Reactive-Extensions/Rx.NET and am pretty sure they do not contain the source of System.Threading.dll from TaskParallelLibrary.

At this point it seems likely that creating a fresh net47-net35 or perhaps corefx/net35 shim from scratch in a single DLL would be cleanest.

@jnm2 jnm2 modified the milestone: 0.2.1 Jan 3, 2018
@jnm2 jnm2 changed the title System.Threading polyfill dependency incorrectly flows SynchronizationContext (net35) System.Threading polyfill dependency incorrectly flows SynchronizationContext Jan 6, 2018
@sharwell
Copy link

Recommend closing all TaskParallelLibrary 1.0.2856 bugs as won't fix with a known workaround. No one has binding redirects in place for this and trying to re-port this is going to be a mess.

@jnm2
Copy link
Collaborator Author

jnm2 commented May 28, 2018

There's another reason to re-port: net20 support. There is a measurable difference between running against the net20 BCL and the net35 and net45 BCLs, and we at NUnit have not been excited about dropping support for old targets, so I'm actually looking for this at the moment.

@jnm2
Copy link
Collaborator Author

jnm2 commented Jun 2, 2018

I have TaskParallelLibrary replaced with .NET Core 2.1 code which enabled me to fix this bug and add a net20 build in https://github.com/OmerMor/AsyncBridge/compare/no_dependencies. Planning for some minor API cleanup, so this will be version 0.4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants