Skip to content
This repository has been archived by the owner on Oct 2, 2024. It is now read-only.

Add support for Task<T>.Result usage #68

Closed
JimiC opened this issue Jan 30, 2016 · 7 comments
Closed

Add support for Task<T>.Result usage #68

JimiC opened this issue Jan 30, 2016 · 7 comments

Comments

@JimiC
Copy link
Contributor

JimiC commented Jan 30, 2016

Currently the SDK doesn't support the use of the Result property of an async Task<T>.
When used it results to a deadlock.
Consider the use of ConfigureAwait(false) where applicable (mainly everywhere).

P.S.: When I used ConfigureAwait(false) in all steps of a GetAsync() call it worked. I know this isn't an easy undertaking but it should be done eventually.

Justification: https://youtu.be/H4EmfpsYcfs?t=3966

@JimiC JimiC changed the title Add support of Task<T>.Result usage Add support for Task<T>.Result usage Jan 30, 2016
@JimiC
Copy link
Contributor Author

JimiC commented Feb 6, 2016

@ginach
Any updates on this? I would really like to see this fixed and released so to remove the special code I'm using to make it work.

@ginach
Copy link
Contributor

ginach commented Feb 9, 2016

Thanks for reporting this. We need to talk this over internally to figure out the right thing to do here but feel free to submit a PR if you have specific changes in mind.

@ificator
Copy link

ificator commented Apr 6, 2016

Since this library can be invoked from UI threads, but does not really care about the SynchronizationContext (i.e. any thread pool thread is fine - we don't need the callbacks on the UI thread), we really should be using ConfigureAwait(false).

@JimiC
Copy link
Contributor Author

JimiC commented Apr 6, 2016

I meant to provide a PR for this but got involved in other projects, which kept me really busy.

@ginach
Copy link
Contributor

ginach commented Apr 6, 2016

Unfortunately, this change will require changes to our code generation logic otherwise changes will be overwritten the next time we generate from the service.

We do want to support this. I had to focus on the MS Graph SDK for a bit and hadn't been able to address the issue here until now. We're actually planning a V2 of the SDK to take in some breaking changes and I had been planning on making this fix then. That said, I can probably make this change sooner since it's not breaking.

As an aside, if the OneDrive functionality you're using is currently supported by MS Graph you might want to check out that library. I made sure to build Task.Result support into it from the beginning.

@JimiC
Copy link
Contributor Author

JimiC commented Apr 6, 2016

Fare enough.
From what I saw on MS Graph I'm afraid it lacks the functionality we need for the project.
Never the less, this issue isn't a blocker atm as I have dumped the flow to another thread so to not block the UI thread.
On top of that we are atm on "Escrow" mode (as you there in MS call it) as we are about to release the next version.
Still glad to see that this is taken under consideration for V2.

@baywet
Copy link
Member

baywet commented Oct 2, 2024

Thank you for reaching out and for your patience. This SDK is being officially deprecated. See #259 for more information

@baywet baywet closed this as not planned Won't fix, can't repro, duplicate, stale Oct 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants