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

Execute and ExecuteAsync freeze when using Service Account #606

Closed
CSurieux opened this issue Oct 4, 2015 · 40 comments
Closed

Execute and ExecuteAsync freeze when using Service Account #606

CSurieux opened this issue Oct 4, 2015 · 40 comments
Assignees
Labels
type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Milestone

Comments

@CSurieux
Copy link

CSurieux commented Oct 4, 2015

Config: all nugets for 1.9.3, even the one not visible from Visual Studio directly downloaded.
Last Calendar available which is 1.9.2.142.
I tried everything to connect on a fresh calendar: API Key, Cliend ID, Server Account. Nothing has been working. I correctly get the token but as soon as I try

EventsResource.InsertRequest request = service.Events.Insert(ev, calendarSettings.GoogleCalendarIds);
Event ev2 =request.Execute();

the execution freeze on Execute, nothing is sent.

So I reverted to 1.9.2 and there, the code is reduced, the only way to have it working is with ServerAccount using P12 certificate.

Has anybody succeeded with calendar and 1.9.3 ??? With which assembly config ?

@CSurieux
Copy link
Author

CSurieux commented Oct 4, 2015

Ok may be it's not the appropriate place for this, I go on Stack Overflow.

@CSurieux CSurieux changed the title Can't have the nugets packages for 19.3 working with calendar Can't have the nugets packages for 1.9.3 working with calendar Oct 4, 2015
@jtattermusch
Copy link
Contributor

Thanks for reporting this.
What visual studio version were you using? What project type (MVC, console app, ...)?
Could you provide a full example with sources and project files?

@CSurieux
Copy link
Author

I am using VS2015 entreprise in an Orchard CMS project (MVC like) using framework 4.51 (we expect to go 4.5.2 soon).
I will have some github repo to show code in one week I hope.
I fear that the problem could be created by the mismatch in libs with the 1.9.3 version.

@CSurieux
Copy link
Author

I made a small project, simply MVC, not Orchard and I have same problem.
VS 2015 project
Do note that this VS2015 project works if I take v 1.9.2.

@jtattermusch
Copy link
Contributor

Have you tried this after #604 was fixed? It looks like this could be caused by the same issue.

@CSurieux
Copy link
Author

Yes I reloaded all the packages after cleaning my project. The assembly dependencies are solved but not my problem.
I don't understand why the same code ( Service Account with P12 certfificate) works with 1.9.2 and not with 1.9.3.
Could it be a pb due to my Windows 10 dev station ? It seems more a pb in an async method not working as expected, as stated in this stackoverflow thread
http://stackoverflow.com/questions/32931997/calendar-not-working-with-version-1-9-3/33067928?noredirect=1#comment53973370_33067928

@jtattermusch
Copy link
Contributor

@CSurieux , it looks like PR #612 could be fixing the problem. Do you have a way to verify this?

@jtattermusch jtattermusch added the type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. label Oct 16, 2015
@CSurieux
Copy link
Author

I try thanks

@CSurieux
Copy link
Author

I am blocked by Visual Studio refusing this constructor
public CalendarService(Apis.Services.BaseClientService.Initializer initializer);

It complains that the type BaseClientService.Initializer is defined in another assembly : Google.Api 1.9.2 , even with a reference to Google.Api 1.9.3 which contains the definition for BaseClientService.Initializer ????

I directly attached the Google VS projects to my project and took the references from the solution.

Severity Code Description Project File Line Source
Error CS0012 The type 'BaseClientService.Initializer' is defined in an assembly that is not referenced. You must add a reference to assembly 'Google.Apis, Version=1.9.2.27817, Culture=neutral, PublicKeyToken=4b01fa6e34db77ab'. WebRefGoogleAPI C:\Users\User\Documents\Visual Studio 2015\Projects\WebRefGoogleAPI\WebRefGoogleAPI\Services\CalendarService.cs 55 IntelliSense

Any idea ?

@CSurieux
Copy link
Author

As I can't see how to use the GitHub cloned code to generate the correct assemblies for my .Net 4.51 project, I reverted to using the Nuget package for 1.9.3 and overrided the methods modified by PR #612 in a derived class to add the ConfigureAwait(false);
No success. The execution never runs out of

return await base.GetAccessTokenForRequestAsync(authUri, cancellationToken).ConfigureAwait(false);

Fiddler shows me that the OAuth2 token is received....

2 little requests:

made virtual 'public async Task InterceptAsync'
suppress the warning on 'public class Initializer' for Service Account

Still expecting some fix, thanks

@peleyal
Copy link
Collaborator

peleyal commented Oct 17, 2015

CSurieux, I'm sorry to hear that you are still struggling with a solution...

I would try to use the latest GitHub assemblies (you will have to build them yourself), and change the app.config assemblies section, as in https://github.com/google/google-api-dotnet-client-samples/blob/master/Tasks.CreateTasks/app.config#L41

Just remember to include your own version number,
bindingRedirect oldVersion="0.0.0.0-1.9.3.xx" newVersion="1.9.3.xx"

I think it should solve the issue you mentioned before
"It complains that the type BaseClientService.Initializer is defined in another assembly : Google.Api 1.9.2 , even with a reference to Google.Api 1.9.3 which contains the definition for BaseClientService.Initializer ????"

Please let us know.
Thanks and good luck!
Eyal

@CSurieux
Copy link
Author

Well, I think that I have found the bug in the code.
Calling all these ConfigureAwait when the original call is on a thread UI seems to loose the orginal thread and things go wrong.
This is the case for me: I call this from a controller action.

I don't know exactly how to solve this because it could depend on local threading conditions.
Good luck :)

@peleyal
Copy link
Collaborator

peleyal commented Oct 17, 2015

It should work for you since there are already MVC and WPF samples, that
count on coming back to the original thread.
Take a look:
https://github.com/google/google-api-dotnet-client-samples/tree/master/Tasks.WPF.ListTasks
and
https://github.com/peleyal/peleyal/tree/master/Google.Apis.Sample.MVC

Just to confirm, you are creating a ASP.NET MVC with Service Account?

Thanks,
Eyal

On Sat, Oct 17, 2015 at 6:14 PM CSurieux notifications@github.com wrote:

Well, I think that I have found the bug in the code.
Calling all these ConfigureAwait when the original call is on a thread UI
seems to loose the orginal thread and things go wrong.
This is the case for me: I call this from a controller action.

I don't know exactly how to solve this because it could depend on local
threading conditions.
Good luck :)


Reply to this email directly or view it on GitHub
#606 (comment)
.

Eyal
peleyal.tumblr.com

@CSurieux
Copy link
Author

Isn't this MVC code is 2 years old ? Does it works with package 1.9.3, I am not sure.
What I see is that when I inherit from ServiceAccount as in

public class MyServiceAccountCredential : ServiceAccountCredential

and do some duplicating for private methods then rewrite this as

    public override async Task<string> GetAccessTokenForRequestAsync(string authUri = null,                          CancellationToken cancellationToken = default(CancellationToken)) {
       if (!HasScopes && authUri != null) {
            // TODO(jtattermusch): support caching of JWT access tokens per authUri, currently a new 
            // JWT access token is created each time, which can hurt performance.
            return CreateJwtAccessToken(authUri);
        }
        string token = GetAccessTokenForRequest2Async(authUri, cancellationToken).Result;
        return token;            
    }

where GetAccessTokenForRequest2Async is just a duplication of the ServiceCredential.GetAccessTokenForRequestAsync method.

It works.
Hope it will help solve the threading pb.

@peleyal
Copy link
Collaborator

peleyal commented Oct 17, 2015

I didn't check that with 1.9.3, sorry...
Jan, it seems that this method was actually added in 1.9.3, can you check
that?
CSurieus, how can we reproduce it? Do you have a short sample code or
something that you can share?

Thanks,

On Sat, Oct 17, 2015 at 6:48 PM CSurieux notifications@github.com wrote:

Isn't this MVC code is 2 years old ? Does it works with package 1.9.3, I
am not sure.
What I see is that when I inherits from Serviec Account as in

public class MyServiceAccountCredential : ServiceAccountCredential

and do some duplicating for private methods the rewrite this as

public override async Task<string> GetAccessTokenForRequestAsync(string authUri = null,                          CancellationToken cancellationToken = default(CancellationToken)) {
   if (!HasScopes && authUri != null) {
        // TODO(jtattermusch): support caching of JWT access tokens per authUri, currently a new
        // JWT access token is created each time, which can hurt performance.
        return CreateJwtAccessToken(authUri);
    }
    string token = GetAccessTokenForRequest2Async(authUri, cancellationToken).Result;
    return token;
}

It works.
Hope it will help solve the threading pb.


Reply to this email directly or view it on GitHub
#606 (comment)
.

Eyal
peleyal.tumblr.com

@peleyal
Copy link
Collaborator

peleyal commented Oct 17, 2015

CSurieus, can you also try to change the following line
https://github.com/google/google-api-dotnet-client/blob/master/Src/GoogleApis.Auth.DotNet4/OAuth2/ServiceAccountCredential.cs#L199
to:
return base.GetAccessTokenForRequestAsync(authUri, cancellationToken);

I believe it might solve it. It is worth giving it a try.

Thanks again,
Eyal

@CSurieux
Copy link
Author

From what I read here, too many async/await/ConfigureAwait cascaded methods is playing with hell in MVC
http://stackoverflow.com/questions/13489065/best-practice-to-call-configureawait-for-all-server-side-code

@CSurieux
Copy link
Author

Replacing as you suggest is not valid for compiler.

@jtattermusch
Copy link
Contributor

calling await base.GetAccessTokenForRequestAsync(authUri, cancellationToken).ConfigureAwait(false); is definitely what should be done in library code.

string token = GetAccessTokenForRequest2Async(authUri, cancellationToken).Result; is an antipattern on the other hand.

The problem is somewhere else in the code, supposedly where the top-most level await is used with ConfigureAwait(false) - the user code that invokes the client library from the MVC controller should decide if the async call should resume in the same synchronization context, the library has no way of knowing that.

@peleyal
Copy link
Collaborator

peleyal commented Oct 20, 2015

Jan, I'm still not sure why did it work before 1.9.3.

CSurieux, Can you also attach the calling code? Do you use ConfigureAwait(false) or not? I think you shouldn't by the way...

Thanks!

@CSurieux
Copy link
Author

string token = GetAccessTokenForRequest2Async(authUri, cancellationToken).Result; is an antipattern on the other hand
the problem is somewhere else in the code

I totally agree but that's the only way I found to solve the issue for me. And I have something to deliver asap.

My working code is just as indicated before, using

string token = GetAccessTokenForRequest2Async(authUri, cancellationToken).Result;

Which I agree is bad.
GetAccessTokenForRequest2Async is just a way to duplicate the GetAccessTokenForRequestAsync in the base.base class of my derived class, I have exactly the same code inside it. Just to be able to step inside with debuger.

@jtattermusch
Copy link
Contributor

Can you provide the callstack at the point where GetAccessTokenForRequest gets invoked?
I am pretty sure there will be an incorrectly configured await somewhere higher up the stack. Also, there will probably be a synchronous wait (.Result or .Wait()) somewhere, which would be the cause of the hang you've been seeing before. Possible explanation:
On the controller thread, an async action is invoked and a sync wait (.Result) is used. That takes up the controller thread. A child action invokes await without using ConfigureAwait(false), which will try to resume on the controller thread that is already taken by the parent action. That causes the hang.

@CSurieux
Copy link
Author

Sorry I was down for a long time due to crashed ssd on another project

@peleyal
Copy link
Collaborator

peleyal commented Oct 31, 2015

No worries. Any update on this one? We currently trying to solve it, but we need your help, since we can't really reproduce it.

@CSurieux
Copy link
Author

CSurieux commented Nov 1, 2015

My code is an Orchard CMS module, I could share it on Bitbucket, would it be of any help for you ?

@peleyal
Copy link
Collaborator

peleyal commented Nov 10, 2015

CSurieux, please share your code, it can be very helpful...

Jan, Do you think that http://stackoverflow.com/questions/33618284/google-calendar-api-not-returning-from-execute-c-sharp is related to this problem?

Thanks,
Eyal

@peleyal peleyal added this to the 1.10 milestone Nov 10, 2015
@peleyal
Copy link
Collaborator

peleyal commented Nov 12, 2015

Another issue with Service Account:
http://stackoverflow.com/questions/33445453/google-api-client-library-freezing-up-in-iis-when-making-request#comment54981726_33445453

We should investigate it before releasing 1.10

@peleyal
Copy link
Collaborator

peleyal commented Nov 12, 2015

@peleyal peleyal changed the title Can't have the nugets packages for 1.9.3 working with calendar Execute and ExecuteAsync freeze when using Service Account Nov 12, 2015
@eddieyanez-immediate
Copy link

Just to confirm that reverting the code back to 1.9.2 fixes for me as well. Anyone trying to also never the code will need to all related dependentAssembly from any app.config AND web.config files as bindingRedirect will try and force to use 1.9.3 if once upgraded.

Re: http://stackoverflow.com/questions/33673536/no-longer-able-to-use-the-apis-both-analytics-getrequest-execute-or-directory

@peleyal
Copy link
Collaborator

peleyal commented Nov 12, 2015

Do you use an installed app auth scenario?
Do you use console app?

I just want to reproduce it myself, so I can start working on fixing it.

Thanks,
Eyal

On Thu, Nov 12, 2015, 1:25 PM eddieyanez-immediate notifications@github.com
wrote:

Just to confirm that reverting the code back to 1.9.2 fixes for me as
well. Anyone trying to also never the code will need to all related
dependentAssembly from any app.config AND web.config files as
bindingRedirect will try and force to use 1.9.3 if once upgraded.

Re:
http://stackoverflow.com/questions/33673536/no-longer-able-to-use-the-apis-both-analytics-getrequest-execute-or-directory


Reply to this email directly or view it on GitHub.

@peleyal
Copy link
Collaborator

peleyal commented Nov 12, 2015

I just ran https://github.com/google/google-api-dotnet-client-samples/tree/master/Plus.ServiceAccount sample and it worked fine for me. I also created a ASP.NET sample that uses ServiceAccount and it works as well. Apparently I'm missing something. Please help me in reproducing it.

Any sample code that currently stuck will be helpful.

Thanks

@eddieyanez-immediate
Copy link

Create a new ASP.Net Web app using MVC and then install the Google.Apis.Admin.Directory_v1 Client library NuGet package. I will also install versions 1.9.2 of Google APIs Auth Client Library, Google APIs Client Library, and Google APIs Core Client Library.

Add the following code inside a controller:

var isAllowed = GoogleGroupMembershipAuthorisationHelper.Authorise(emailAddress);

Here's a sample code for GoogleGroupMembershipAuthorisationHelper:

public static class GoogleGroupMembershipAuthorisationHelper
{

    public static bool Authorise(string userEmailAddress)
    {
        var certificateFilePath = HostingEnvironment.MapPath("~/xxx/yyy.p12");

        // create credential
        var certificate = new X509Certificate2(certificateFilePath, "notasecret", X509KeyStorageFlags.MachineKeySet | X509KeyStorageFlags.PersistKeySet | X509KeyStorageFlags.Exportable);

        var credential = new ServiceAccountCredential(
            new ServiceAccountCredential.Initializer("zzz@developer.gserviceaccount.com")
            {
                Scopes = new[] { DirectoryService.Scope.AdminDirectoryGroupMember },
                User = "user-aaaa@email.com"
            }.FromCertificate(certificate));

        var service = new DirectoryService(new BaseClientService.Initializer
        {
            HttpClientInitializer = credential,
            ApplicationName = "bbb",
        });
        var getRequest = service.Members.List("ccc@email.com");

        var result = getRequest.Execute();

        return result.MembersValue.Any(x => x.Email.Equals(userEmailAddress, StringComparison.InvariantCultureIgnoreCase));
    }

}

Run the solution, and the code should work fine.

Then fire up the NuGet management window and go take a look under Updates. Google APIs Auth Client Library, Google APIs Client Library, and Google APIs Core Client Library will be listed. Update all three (these will be updated to versions 1.9.3).

Run the solution. The code execution will hang when this line is called:

var result = getRequest.Execute();

@peleyal
Copy link
Collaborator

peleyal commented Nov 13, 2015

OK. Good news everyone!
I played with it and I found the following:

  1. In 1.9.2 everything works as expected.
  2. In 1.9.3 the method stuck
  3. I tried to use the latest code in github and it works. Both Jan and I chatted yesterday, and we think that this PR Adding ConfigureAwait(false) where missing #612 fixed the issue.
  4. I tried both ExecuteAsync and Execute and both methods worked for me.
  5. Then, just to verify, I changed the local code to "rollback" Adding ConfigureAwait(false) where missing #612, ExecuteAsync still worked for me, but Execute didn't.
  6. So, I think we are good.

eddieyanez-immediate@ nad others, I would be happy to see if you can check the latest version, by running the following:

  1. Compile the github Google Apis projects from head.
  2. Download https://developers.google.com/resources/api-libraries/download/admin/directory_v1/csharp. the zip file contains a src folder with your DirectoryService (downloading the code and compile it yourself will be better than playing with binding redirects).
  3. Create a new project and add the https://www.nuget.org/packages/Google.Apis.Admin.Directory.directory_v1/ NuGet package (with all its dependencies)
  4. Delete all existing reference to Google.Apis* (including Google.Apis, Google.Apis.Core, Google.Apis.Auth, Google.Apis.Auth.PlatformServices, Google.Apis.PlatformsServices and Google.Apis.Admin.Directory_v1)
  5. Add back the references, but this time use the versions you build from the repository (step 1)
  6. Add the directory file you downloaded in step 2.
  7. Now add the code in your controller.
  8. Compile & Run, hopefully it works 👍

Please verify it, then we can close this issue, and start working on new release that fixes this issue.

Thanks!
Eyal

@jtattermusch
Copy link
Contributor

@peleyal Thanks Eyal for taking the time to test this thoroughly! Should we close this issue now?

@peleyal
Copy link
Collaborator

peleyal commented Nov 18, 2015

I prefer to leave it open for the next few days. I'm still waiting for someone (besides me) to test that it works as expected.

@eddieyanez-immediate
Copy link

Sorry guys, I've been caught up at work this week. I'll be testing this properly on Friday.

@peleyal
Copy link
Collaborator

peleyal commented Nov 18, 2015

No worries. Please keep us updated

@peleyal
Copy link
Collaborator

peleyal commented Nov 24, 2015

I'm closing this one. No one complained about it.

I checked it myself and it seems to work now as I commented 11 days ago.
If anyone wants to check that before the coming 1.10 release (which is planned for the next few weeks) I would be more than happy 👍

@CSurieux
Copy link
Author

Well done 1.10 solved my problem and I could reverse all the work around I had installed.

@peleyal
Copy link
Collaborator

peleyal commented Dec 28, 2015

Good to hear, thank you for keeping us updated!
Eyal

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

4 participants